diff --git a/.gitignore b/.gitignore index 493df5a7fd..8fb170c30f 100644 --- a/.gitignore +++ b/.gitignore @@ -28,4 +28,6 @@ nosetests.xml cover_html/ .idea/ .redcar/ -chromedriver.log \ No newline at end of file +chromedriver.log +/nbproject +ghostdriver.log diff --git a/.pylintrc b/.pylintrc index ce2f2e3b87..d1cdbb4780 100644 --- a/.pylintrc +++ b/.pylintrc @@ -12,7 +12,7 @@ profile=no # Add files or directories to the blacklist. They should be base names, not # paths. -ignore=CVS +ignore=CVS, migrations # Pickle collected data for later comparisons. persistent=yes @@ -33,7 +33,11 @@ load-plugins= # can either give multiple identifier separated by comma (,) or put this option # multiple time (only on the command line, not in the configuration file where # it should appear only once). -disable=E1102,W0142 +disable= +# W0141: Used builtin function 'map' +# W0142: Used * or ** magic +# R0903: Too few public methods (1/2) + W0141,W0142,R0903 [REPORTS] @@ -43,7 +47,7 @@ disable=E1102,W0142 output-format=text # Include message's id in output -include-ids=no +include-ids=yes # Put messages in a separate file for each module / package specified on the # command line instead of printing them on stdout. Reports (if any) will be @@ -97,7 +101,7 @@ bad-functions=map,filter,apply,input module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ # Regular expression which should only match correct module level names -const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__))$ +const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__)|log|urlpatterns)$ # Regular expression which should only match correct class names class-rgx=[A-Z_][a-zA-Z0-9]+$ @@ -106,7 +110,7 @@ class-rgx=[A-Z_][a-zA-Z0-9]+$ function-rgx=[a-z_][a-z0-9_]{2,30}$ # Regular expression which should only match correct method names -method-rgx=[a-z_][a-z0-9_]{2,30}$ +method-rgx=([a-z_][a-z0-9_]{2,60}|setUp|set[Uu]pClass|tearDown|tear[Dd]ownClass|assert[A-Z]\w*)$ # Regular expression which should only match correct instance attribute names attr-rgx=[a-z_][a-z0-9_]{2,30}$ diff --git a/.ruby-version b/.ruby-version index dd472cffa2..311baaf3e2 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -1.8.7-p371 \ No newline at end of file +1.9.3-p374 diff --git a/Gemfile b/Gemfile index 43a9f6e2b1..7f7b146978 100644 --- a/Gemfile +++ b/Gemfile @@ -1,4 +1,4 @@ -source :rubygems +source 'https://rubygems.org' gem 'rake', '~> 10.0.3' gem 'sass', '3.1.15' gem 'bourbon', '~> 1.3.6' diff --git a/cms/djangoapps/contentstore/features/advanced-settings.feature b/cms/djangoapps/contentstore/features/advanced-settings.feature index 4708a60be1..779d44e4b2 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.feature +++ b/cms/djangoapps/contentstore/features/advanced-settings.feature @@ -7,6 +7,7 @@ Feature: Advanced (manual) course policy When I select the Advanced Settings Then I see only the display name + @skip-phantom Scenario: Test if there are no policy settings without existing UI controls Given I am on the Advanced Course Settings page in Studio When I delete the display name @@ -14,6 +15,7 @@ Feature: Advanced (manual) course policy And I reload the page Then there are no advanced policy settings + @skip-phantom Scenario: Test cancel editing key name Given I am on the Advanced Course Settings page in Studio When I edit the name of a policy key @@ -32,6 +34,7 @@ Feature: Advanced (manual) course policy And I press the "Cancel" notification button Then the policy key value is unchanged + @skip-phantom Scenario: Test editing key value Given I am on the Advanced Course Settings page in Studio When I edit the value of a policy key diff --git a/cms/djangoapps/contentstore/features/advanced-settings.py b/cms/djangoapps/contentstore/features/advanced-settings.py index 91daf70718..1024579b45 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.py +++ b/cms/djangoapps/contentstore/features/advanced-settings.py @@ -1,9 +1,10 @@ from lettuce import world, step from common import * import time +from selenium.common.exceptions import WebDriverException +from selenium.webdriver.support import expected_conditions as EC -from nose.tools import assert_equal -from nose.tools import assert_true +from nose.tools import assert_true, assert_false, assert_equal """ http://selenium.googlecode.com/svn/trunk/docs/api/py/webdriver/selenium.webdriver.common.keys.html @@ -19,6 +20,7 @@ def i_select_advanced_settings(step): css_click(expand_icon_css) link_css = 'li.nav-course-settings-advanced a' css_click(link_css) + # world.browser.click_link_by_text('Advanced Settings') @step('I am on the Advanced Course Settings page in Studio$') @@ -37,13 +39,25 @@ def reload_the_page(step): def edit_the_name_of_a_policy_key(step): policy_key_css = 'input.policy-key' e = css_find(policy_key_css).first - e.fill('new') + e.type('_new') @step(u'I press the "([^"]*)" notification button$') def press_the_notification_button(step, name): - world.browser.click_link_by_text(name) + def is_visible(driver): + return EC.visibility_of_element_located((By.CSS_SELECTOR,css,)) + def is_invisible(driver): + return EC.invisibility_of_element_located((By.CSS_SELECTOR,css,)) + css = 'a.%s-button' % name.lower() + wait_for(is_visible) + + try: + css_click_at(css) + wait_for(is_invisible) + except WebDriverException, e: + css_click_at(css) + wait_for(is_invisible) @step(u'I edit the value of a policy key$') def edit_the_value_of_a_policy_key(step): @@ -83,7 +97,12 @@ def i_see_only_display_name(step): @step('there are no advanced policy settings$') def no_policy_settings(step): - assert_policy_entries([], []) + keys_css = 'input.policy-key' + val_css = 'textarea.json' + k = world.browser.is_element_not_present_by_css(keys_css, 5) + v = world.browser.is_element_not_present_by_css(val_css, 5) + assert_true(k) + assert_true(v) @step('they are alphabetized$') @@ -99,29 +118,29 @@ def it_is_formatted(step): @step(u'the policy key name is unchanged$') def the_policy_key_name_is_unchanged(step): policy_key_css = 'input.policy-key' - e = css_find(policy_key_css).first - assert_equal(e.value, 'display_name') + val = css_find(policy_key_css).first.value + assert_equal(val, 'display_name') @step(u'the policy key name is changed$') def the_policy_key_name_is_changed(step): policy_key_css = 'input.policy-key' - e = css_find(policy_key_css).first - assert_equal(e.value, 'new') + val = css_find(policy_key_css).first.value + assert_equal(val, 'display_name_new') @step(u'the policy key value is unchanged$') def the_policy_key_value_is_unchanged(step): policy_value_css = 'li.course-advanced-policy-list-item div.value textarea' - e = css_find(policy_value_css).first - assert_equal(e.value, '"Robot Super Course"') + val = css_find(policy_value_css).first.value + assert_equal(val, '"Robot Super Course"') @step(u'the policy key value is changed$') def the_policy_key_value_is_unchanged(step): policy_value_css = 'li.course-advanced-policy-list-item div.value textarea' - e = css_find(policy_value_css).first - assert_equal(e.value, '"Robot Super Course X"') + val = css_find(policy_value_css).first.value + assert_equal(val, '"Robot Super Course X"') ############# HELPERS ############### @@ -132,19 +151,23 @@ def create_entry(key, value): new_key_css = 'div#__new_advanced_key__ input' new_key_element = css_find(new_key_css).first new_key_element.fill(key) -# For some reason have to get the instance for each command (get error that it is no longer attached to the DOM) -# Have to do all this because Selenium has a bug that fill does not remove existing text +# For some reason have to get the instance for each command +# (get error that it is no longer attached to the DOM) +# Have to do all this because Selenium fill does not remove existing text new_value_css = 'div.CodeMirror textarea' css_find(new_value_css).last.fill("") css_find(new_value_css).last._element.send_keys(Keys.DELETE, Keys.DELETE) css_find(new_value_css).last.fill(value) + # Add in a TAB key press because intermittently on ubuntu the + # last character of "value" above was not getting typed in + css_find(new_value_css).last._element.send_keys(Keys.TAB) def delete_entry(index): """ Delete the nth entry where index is 0-based """ - css = '.delete-button' + css = 'a.delete-button' assert_true(world.browser.is_element_present_by_css(css, 5)) delete_buttons = css_find(css) assert_true(len(delete_buttons) > index, "no delete button exists for entry " + str(index)) @@ -152,8 +175,8 @@ def delete_entry(index): def assert_policy_entries(expected_keys, expected_values): - assert_entries('.key input', expected_keys) - assert_entries('.json', expected_values) + assert_entries('.key input.policy-key', expected_keys) + assert_entries('textarea.json', expected_values) def assert_entries(css, expected_values): @@ -165,16 +188,8 @@ def assert_entries(css, expected_values): def click_save(): - css = ".save-button" - - def is_shown(driver): - visible = css_find(css).first.visible - if visible: - # Even when waiting for visible, this fails sporadically. Adding in a small wait. - time.sleep(float(1)) - return visible - wait_for(is_shown) - css_click(css) + css = "a.save-button" + css_click_at(css) def fill_last_field(value): diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index 61b4fee9f6..2ec0427e1d 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -3,18 +3,20 @@ from lettuce.django import django_url from nose.tools import assert_true from nose.tools import assert_equal from selenium.webdriver.support.ui import WebDriverWait +from selenium.common.exceptions import WebDriverException, StaleElementReferenceException +from selenium.webdriver.support import expected_conditions as EC +from selenium.webdriver.common.by import By from terrain.factories import UserFactory, RegistrationFactory, UserProfileFactory from terrain.factories import CourseFactory, GroupFactory -import xmodule.modulestore.django +from xmodule.modulestore.django import _MODULESTORES, modulestore +from xmodule.templates import update_templates from auth.authz import get_user_by_email from logging import getLogger logger = getLogger(__name__) ########### STEP HELPERS ############## - - @step('I (?:visit|access|open) the Studio homepage$') def i_visit_the_studio_homepage(step): # To make this go to port 8001, put @@ -52,9 +54,8 @@ def i_have_opened_a_new_course(step): log_into_studio() create_a_course() + ####### HELPER FUNCTIONS ############## - - def create_studio_user( uname='robot', email='robot+studio@edx.org', @@ -83,9 +84,9 @@ def flush_xmodule_store(): # (though it shouldn't), do this manually # from the bash shell to drop it: # $ mongo test_xmodule --eval "db.dropDatabase()" - xmodule.modulestore.django._MODULESTORES = {} - xmodule.modulestore.django.modulestore().collection.drop() - xmodule.templates.update_templates() + _MODULESTORES = {} + modulestore().collection.drop() + update_templates() def assert_css_with_text(css, text): @@ -94,8 +95,16 @@ def assert_css_with_text(css, text): def css_click(css): - assert_true(world.browser.is_element_present_by_css(css, 5)) - world.browser.find_by_css(css).first.click() + ''' + First try to use the regular click method, + but if clicking in the middle of an element + doesn't work it might be that it thinks some other + element is on top of it there so click in the upper left + ''' + try: + css_find(css).first.click() + except WebDriverException, e: + css_click_at(css) def css_click_at(css, x=10, y=10): @@ -103,8 +112,7 @@ def css_click_at(css, x=10, y=10): A method to click at x,y coordinates of the element rather than in the center of the element ''' - assert_true(world.browser.is_element_present_by_css(css, 5)) - e = world.browser.find_by_css(css).first + e = css_find(css).first e.action_chains.move_to_element_with_offset(e._element, x, y) e.action_chains.click() e.action_chains.perform() @@ -115,11 +123,16 @@ def css_fill(css, value): def css_find(css): + def is_visible(driver): + return EC.visibility_of_element_located((By.CSS_SELECTOR,css,)) + + world.browser.is_element_present_by_css(css, 5) + wait_for(is_visible) return world.browser.find_by_css(css) def wait_for(func): - WebDriverWait(world.browser.driver, 10).until(func) + WebDriverWait(world.browser.driver, 5).until(func) def id_find(id): diff --git a/cms/djangoapps/contentstore/features/section.feature b/cms/djangoapps/contentstore/features/section.feature index 75e7a4af10..08d38367bc 100644 --- a/cms/djangoapps/contentstore/features/section.feature +++ b/cms/djangoapps/contentstore/features/section.feature @@ -26,9 +26,10 @@ Feature: Create Section And I save a new section release date Then the section release date is updated + @skip-phantom Scenario: Delete section Given I have opened a new course in Studio And I have added a new section When I press the "section" delete icon And I confirm the alert - Then the section does not exist \ No newline at end of file + Then the section does not exist diff --git a/cms/djangoapps/contentstore/features/section.py b/cms/djangoapps/contentstore/features/section.py index cfa4e4bb52..b5ddb48a09 100644 --- a/cms/djangoapps/contentstore/features/section.py +++ b/cms/djangoapps/contentstore/features/section.py @@ -1,6 +1,8 @@ from lettuce import world, step from common import * from nose.tools import assert_equal +from selenium.webdriver.common.keys import Keys +import time ############### ACTIONS #################### @@ -37,10 +39,14 @@ def i_save_a_new_section_release_date(step): date_css = 'input.start-date.date.hasDatepicker' time_css = 'input.start-time.time.ui-timepicker-input' css_fill(date_css, '12/25/2013') - # click here to make the calendar go away - css_click(time_css) + # hit TAB to get to the time field + e = css_find(date_css).first + e._element.send_keys(Keys.TAB) css_fill(time_css, '12:00am') - css_click('a.save-button') + e = css_find(time_css).first + e._element.send_keys(Keys.TAB) + time.sleep(float(1)) + world.browser.click_link_by_text('Save') ############ ASSERTIONS ################### @@ -106,7 +112,7 @@ def the_section_release_date_picker_not_visible(step): def the_section_release_date_is_updated(step): css = 'span.published-status' status_text = world.browser.find_by_css(css).text - assert status_text == 'Will Release: 12/25/2013 at 12:00am' + assert_equal(status_text,'Will Release: 12/25/2013 at 12:00am') ############ HELPER METHODS ################### @@ -120,4 +126,4 @@ def save_section_name(name): def see_my_section_on_the_courseware_page(name): section_css = 'span.section-name-span' - assert_css_with_text(section_css, name) \ No newline at end of file + assert_css_with_text(section_css, name) diff --git a/cms/djangoapps/contentstore/features/signup.py b/cms/djangoapps/contentstore/features/signup.py index a786225ead..e8d0dd8229 100644 --- a/cms/djangoapps/contentstore/features/signup.py +++ b/cms/djangoapps/contentstore/features/signup.py @@ -1,4 +1,5 @@ from lettuce import world, step +from common import * @step('I fill in the registration form$') @@ -13,10 +14,11 @@ def i_fill_in_the_registration_form(step): @step('I press the Create My Account button on the registration form$') def i_press_the_button_on_the_registration_form(step): - register_form = world.browser.find_by_css('form#register_form') - submit_css = 'button#submit' - register_form.find_by_css(submit_css).click() - + submit_css = 'form#register_form button#submit' + # Workaround for click not working on ubuntu + # for some unknown reason. + e = css_find(submit_css) + e.type(' ') @step('I should see be on the studio home page$') def i_should_see_be_on_the_studio_home_page(step): diff --git a/cms/djangoapps/contentstore/features/studio-overview-togglesection.feature b/cms/djangoapps/contentstore/features/studio-overview-togglesection.feature index 5276b90d12..52c10e41a8 100644 --- a/cms/djangoapps/contentstore/features/studio-overview-togglesection.feature +++ b/cms/djangoapps/contentstore/features/studio-overview-togglesection.feature @@ -21,6 +21,7 @@ Feature: Overview Toggle Section Then I see the "Collapse All Sections" link And all sections are expanded + @skip-phantom Scenario: Collapse link is not removed after last section of a course is deleted Given I have a course with 1 section And I navigate to the course overview page diff --git a/cms/djangoapps/contentstore/features/subsection.feature b/cms/djangoapps/contentstore/features/subsection.feature index 4b5f5b869d..1be5f4aeb9 100644 --- a/cms/djangoapps/contentstore/features/subsection.feature +++ b/cms/djangoapps/contentstore/features/subsection.feature @@ -17,6 +17,7 @@ Feature: Create Subsection And I click to edit the subsection name Then I see the complete subsection name with a quote in the editor + @skip-phantom Scenario: Delete a subsection Given I have opened a new course section in Studio And I have added a new subsection diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 8e4a016a0f..9d533dffed 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -9,10 +9,8 @@ from tempdir import mkdtemp_clean import json from fs.osfs import OSFS import copy -from mock import Mock -from json import dumps, loads +from json import loads -from student.models import Registration from django.contrib.auth.models import User from cms.djangoapps.contentstore.utils import get_modulestore @@ -22,12 +20,11 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore import Location from xmodule.modulestore.store_utilities import clone_course from xmodule.modulestore.store_utilities import delete_course -from xmodule.modulestore.django import modulestore, _MODULESTORES +from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore from xmodule.templates import update_templates from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_importer import import_from_xml -from xmodule.templates import update_templates from xmodule.capa_module import CapaDescriptor from xmodule.course_module import CourseDescriptor @@ -63,7 +60,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.client = Client() self.client.login(username=uname, password=password) - def check_edit_unit(self, test_course_name): import_from_xml(modulestore(), 'common/test/data/', [test_course_name]) @@ -82,8 +78,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_static_tab_reordering(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) + module_store = modulestore('direct') + course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) # reverse the ordering reverse_tabs = [] @@ -91,9 +87,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): if tab['type'] == 'static_tab': reverse_tabs.insert(0, 'i4x://edX/full/static_tab/{0}'.format(tab['url_slug'])) - resp = self.client.post(reverse('reorder_static_tabs'), json.dumps({'tabs': reverse_tabs}), "application/json") + self.client.post(reverse('reorder_static_tabs'), json.dumps({'tabs': reverse_tabs}), "application/json") - course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) + course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) # compare to make sure that the tabs information is in the expected order after the server call course_tabs = [] @@ -103,28 +99,60 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertEqual(reverse_tabs, course_tabs) + def test_delete(self): + import_from_xml(modulestore(), 'common/test/data/', ['full']) + + module_store = modulestore('direct') + + sequential = module_store.get_item(Location(['i4x', 'edX', 'full', 'sequential', 'Administrivia_and_Circuit_Elements', None])) + + chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None])) + + # make sure the parent no longer points to the child object which was deleted + self.assertTrue(sequential.location.url() in chapter.definition['children']) + + self.client.post(reverse('delete_item'), + json.dumps({'id': sequential.location.url(), 'delete_children':'true', 'delete_all_versions':'true'}), + "application/json") + + found = False + try: + module_store.get_item(Location(['i4x', 'edX', 'full', 'sequential', 'Administrivia_and_Circuit_Elements', None])) + found = True + except ItemNotFoundError: + pass + + self.assertFalse(found) + + chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None])) + + # make sure the parent no longer points to the child object which was deleted + self.assertFalse(sequential.location.url() in chapter.definition['children']) + + + 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])) + module_store = modulestore('direct') + effort = module_store.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])) + effort = module_store.get_item(Location(['i4x', 'edX', 'full', 'about', 'end_date', None])) self.assertEqual(effort.definition['data'], 'TBD') def test_remove_hide_progress_tab(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') - course = ms.get_item(source_location) + course = module_store.get_item(source_location) self.assertNotIn('hide_progress_tab', course.metadata) def test_clone_course(self): @@ -143,19 +171,19 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): data = parse_json(resp) self.assertEqual(data['id'], 'i4x://MITx/999/course/Robot_Super_Course') - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') dest_location = CourseDescriptor.id_to_location('MITx/999/Robot_Super_Course') - clone_course(ms, cs, source_location, dest_location) + clone_course(module_store, content_store, source_location, dest_location) # now loop through all the units in the course and verify that the clone can render them, which # means the objects are at least present - items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) + items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) self.assertGreater(len(items), 0) - clone_items = ms.get_items(Location(['i4x', 'MITx', '999', 'vertical', None])) + clone_items = module_store.get_items(Location(['i4x', 'MITx', '999', 'vertical', None])) self.assertGreater(len(clone_items), 0) for descriptor in items: new_loc = descriptor.location._replace(org='MITx', course='999') @@ -166,14 +194,14 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_delete_course(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') - delete_course(ms, cs, location, commit=True) + delete_course(module_store, content_store, location, commit=True) - items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) + items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) self.assertEqual(len(items), 0) def verify_content_existence(self, modulestore, root_dir, location, dirname, category_name, filename_suffix=''): @@ -188,10 +216,10 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertTrue(fs.exists(item.location.name + filename_suffix)) def test_export_course(self): - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() - import_from_xml(ms, 'common/test/data/', ['full']) + import_from_xml(module_store, 'common/test/data/', ['full']) location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') root_dir = path(mkdtemp_clean()) @@ -199,43 +227,43 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): print 'Exporting to tempdir = {0}'.format(root_dir) # export out to a tempdir - export_to_xml(ms, cs, location, root_dir, 'test_export') + export_to_xml(module_store, content_store, location, root_dir, 'test_export') # check for static tabs - self.verify_content_existence(ms, root_dir, location, 'tabs', 'static_tab', '.html') + self.verify_content_existence(module_store, root_dir, location, 'tabs', 'static_tab', '.html') # check for custom_tags - self.verify_content_existence(ms, root_dir, location, 'info', 'course_info', '.html') + self.verify_content_existence(module_store, root_dir, location, 'info', 'course_info', '.html') # check for custom_tags - self.verify_content_existence(ms, root_dir, location, 'custom_tags', 'custom_tag_template') + self.verify_content_existence(module_store, root_dir, location, 'custom_tags', 'custom_tag_template') # check for graiding_policy.json fs = OSFS(root_dir / 'test_export/policies/6.002_Spring_2012') self.assertTrue(fs.exists('grading_policy.json')) - course = ms.get_item(location) + course = module_store.get_item(location) # compare what's on disk compared to what we have in our course - with fs.open('grading_policy.json','r') as grading_policy: - on_disk = loads(grading_policy.read()) + with fs.open('grading_policy.json', 'r') as grading_policy: + on_disk = loads(grading_policy.read()) self.assertEqual(on_disk, course.definition['data']['grading_policy']) #check for policy.json self.assertTrue(fs.exists('policy.json')) # compare what's on disk to what we have in the course module - with fs.open('policy.json','r') as course_policy: + with fs.open('policy.json', 'r') as course_policy: on_disk = loads(course_policy.read()) self.assertIn('course/6.002_Spring_2012', on_disk) self.assertEqual(on_disk['course/6.002_Spring_2012'], course.metadata) # remove old course - delete_course(ms, cs, location) + delete_course(module_store, content_store, location) # reimport - import_from_xml(ms, root_dir, ['test_export']) + import_from_xml(module_store, root_dir, ['test_export']) - items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) + items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) self.assertGreater(len(items), 0) for descriptor in items: print "Checking {0}....".format(descriptor.location.url()) @@ -245,11 +273,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): shutil.rmtree(root_dir) def test_course_handouts_rewrites(self): - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() # import a test course - import_from_xml(ms, 'common/test/data/', ['full']) + import_from_xml(module_store, 'common/test/data/', ['full']) handout_location = Location(['i4x', 'edX', 'full', 'course_info', 'handouts']) @@ -263,7 +291,33 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # note, we know the link it should be because that's what in the 'full' course in the test data self.assertContains(resp, '/c4x/edX/full/asset/handouts_schematic_tutorial.pdf') + def test_export_course_with_unknown_metadata(self): + module_store = modulestore('direct') + content_store = contentstore() + import_from_xml(module_store, 'common/test/data/', ['full']) + location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') + + root_dir = path(mkdtemp_clean()) + + course = module_store.get_item(location) + + # add a bool piece of unknown metadata so we can verify we don't throw an exception + course.metadata['new_metadata'] = True + + module_store.update_metadata(location, course.metadata) + + print 'Exporting to tempdir = {0}'.format(root_dir) + + # export out to a tempdir + exported = False + try: + export_to_xml(module_store, content_store, location, root_dir, 'test_export') + exported = True + except Exception: + pass + + self.assertTrue(exported) class ContentStoreTest(ModuleStoreTestCase): """ @@ -402,7 +456,7 @@ class ContentStoreTest(ModuleStoreTestCase): def test_capa_module(self): """Test that a problem treats markdown specially.""" - course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') + CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') problem_data = { 'parent_location': 'i4x://MITx/999/course/Robot_Super_Course', @@ -424,10 +478,10 @@ class ContentStoreTest(ModuleStoreTestCase): def test_import_metadata_with_attempts_empty_string(self): import_from_xml(modulestore(), 'common/test/data/', ['simple']) - ms = modulestore('direct') + module_store = modulestore('direct') did_load_item = False try: - ms.get_item(Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None])) + module_store.get_item(Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None])) did_load_item = True except ItemNotFoundError: pass @@ -438,10 +492,10 @@ class ContentStoreTest(ModuleStoreTestCase): def test_metadata_inheritance(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) + module_store = modulestore('direct') + course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) - verticals = ms.get_items(['i4x', 'edX', 'full', 'vertical', None, None]) + verticals = module_store.get_items(['i4x', 'edX', 'full', 'vertical', None, None]) # let's assert on the metadata_inheritance on an existing vertical for vertical in verticals: @@ -452,15 +506,15 @@ class ContentStoreTest(ModuleStoreTestCase): new_component_location = Location('i4x', 'edX', 'full', 'html', 'new_component') source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') - + # crate a new module and add it as a child to a vertical - ms.clone_item(source_template_location, new_component_location) + module_store.clone_item(source_template_location, new_component_location) parent = verticals[0] - ms.update_children(parent.location, parent.definition.get('children', []) + [new_component_location.url()]) + module_store.update_children(parent.location, parent.definition.get('children', []) + [new_component_location.url()]) # flush the cache - ms.get_cached_metadata_inheritance_tree(new_component_location, -1) - new_module = ms.get_item(new_component_location) + module_store.get_cached_metadata_inheritance_tree(new_component_location, -1) + new_module = module_store.get_item(new_component_location) # check for grace period definition which should be defined at the course level self.assertIn('graceperiod', new_module.metadata) @@ -473,11 +527,11 @@ class ContentStoreTest(ModuleStoreTestCase): # now let's define an override at the leaf node level # new_module.metadata['graceperiod'] = '1 day' - ms.update_metadata(new_module.location, new_module.metadata) + module_store.update_metadata(new_module.location, new_module.metadata) # flush the cache and refetch - ms.get_cached_metadata_inheritance_tree(new_component_location, -1) - new_module = ms.get_item(new_component_location) + module_store.get_cached_metadata_inheritance_tree(new_component_location, -1) + new_module = module_store.get_item(new_component_location) self.assertIn('graceperiod', new_module.metadata) self.assertEqual('1 day', new_module.metadata['graceperiod']) @@ -486,15 +540,15 @@ class ContentStoreTest(ModuleStoreTestCase): class TemplateTestCase(ModuleStoreTestCase): def test_template_cleanup(self): - ms = modulestore('direct') + module_store = modulestore('direct') # insert a bogus template in the store bogus_template_location = Location('i4x', 'edx', 'templates', 'html', 'bogus') source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') - - ms.clone_item(source_template_location, bogus_template_location) - verify_create = ms.get_item(bogus_template_location) + module_store.clone_item(source_template_location, bogus_template_location) + + verify_create = module_store.get_item(bogus_template_location) self.assertIsNotNone(verify_create) # now run cleanup @@ -503,10 +557,9 @@ class TemplateTestCase(ModuleStoreTestCase): # now try to find dangling template, it should not be in DB any longer asserted = False try: - verify_create = ms.get_item(bogus_template_location) + verify_create = module_store.get_item(bogus_template_location) except ItemNotFoundError: asserted = True - self.assertTrue(asserted) - + self.assertTrue(asserted) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 34003d71a4..6566350f8d 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -86,12 +86,14 @@ def signup(request): csrf_token = csrf(request)['csrf_token'] return render_to_response('signup.html', {'csrf': csrf_token}) + def old_login_redirect(request): ''' Redirect to the active login url. ''' return redirect('login', permanent=True) + @ssl_login_shortcut @ensure_csrf_cookie def login_page(request): @@ -104,6 +106,7 @@ def login_page(request): 'forgot_password_link': "//{base}/#forgot-password-modal".format(base=settings.LMS_BASE), }) + def howitworks(request): if request.user.is_authenticated(): return index(request) @@ -112,6 +115,7 @@ def howitworks(request): # ==== Views for any logged-in user ================================== + @login_required @ensure_csrf_cookie def index(request): @@ -145,6 +149,7 @@ def index(request): # ==== Views with per-item permissions================================ + def has_access(user, location, role=STAFF_ROLE_NAME): ''' Return True if user allowed to access this piece of data @@ -393,6 +398,7 @@ def preview_component(request, location): 'editor': wrap_xmodule(component.get_html, component, 'xmodule_edit.html')(), }) + @expect_json @login_required @ensure_csrf_cookie @@ -636,6 +642,17 @@ def delete_item(request): if item.location.revision is None and item.location.category == 'vertical' and delete_all_versions: modulestore('direct').delete_item(item.location) + # cdodge: we need to remove our parent's pointer to us so that it is no longer dangling + if delete_all_versions: + parent_locs = modulestore('direct').get_parent_locations(item_loc, None) + + for parent_loc in parent_locs: + parent = modulestore('direct').get_item(parent_loc) + item_url = item_loc.url() + if item_url in parent.definition["children"]: + parent.definition["children"].remove(item_url) + modulestore('direct').update_children(parent.location, parent.definition["children"]) + return HttpResponse() @@ -709,6 +726,7 @@ def create_draft(request): return HttpResponse() + @login_required @expect_json def publish_draft(request): @@ -738,6 +756,7 @@ def unpublish_unit(request): return HttpResponse() + @login_required @expect_json def clone_item(request): @@ -768,8 +787,7 @@ def clone_item(request): return HttpResponse(json.dumps({'id': dest_location.url()})) -#@login_required -#@ensure_csrf_cookie + def upload_asset(request, org, course, coursename): ''' cdodge: this method allows for POST uploading of files into the course asset library, which will @@ -831,6 +849,7 @@ def upload_asset(request, org, course, coursename): response['asset_url'] = StaticContent.get_url_path_from_location(content.location) return response + ''' This view will return all CMS users who are editors for the specified course ''' @@ -863,6 +882,7 @@ def create_json_response(errmsg = None): return resp + ''' This POST-back view will add a user - specified by email - to the list of editors for the specified course @@ -895,6 +915,7 @@ def add_user(request, location): return create_json_response() + ''' This POST-back view will remove a user - specified by email - from the list of editors for the specified course @@ -926,6 +947,7 @@ def remove_user(request, location): def landing(request, org, course, coursename): return render_to_response('temp-course-landing.html', {}) + @login_required @ensure_csrf_cookie def static_pages(request, org, course, coursename): @@ -1029,6 +1051,7 @@ def edit_tabs(request, org, course, coursename): 'components': components }) + def not_found(request): return render_to_response('error.html', {'error': '404'}) @@ -1064,6 +1087,7 @@ def course_info(request, org, course, name, provided_id=None): 'handouts_location': Location(['i4x', org, course, 'course_info', 'handouts']).url() }) + @expect_json @login_required @ensure_csrf_cookie @@ -1161,6 +1185,7 @@ def get_course_settings(request, org, course, name): "section": "details"}) }) + @login_required @ensure_csrf_cookie def course_config_graders_page(request, org, course, name): @@ -1184,6 +1209,7 @@ def course_config_graders_page(request, org, course, name): 'course_details': json.dumps(course_details, cls=CourseSettingsEncoder) }) + @login_required @ensure_csrf_cookie def course_config_advanced_page(request, org, course, name): @@ -1207,6 +1233,7 @@ def course_config_advanced_page(request, org, course, name): 'advanced_dict' : json.dumps(CourseMetadata.fetch(location)), }) + @expect_json @login_required @ensure_csrf_cookie @@ -1238,6 +1265,7 @@ def course_settings_updates(request, org, course, name, section): return HttpResponse(json.dumps(manager.update_from_json(request.POST), cls=CourseSettingsEncoder), mimetype="application/json") + @expect_json @login_required @ensure_csrf_cookie @@ -1272,7 +1300,7 @@ def course_grader_updates(request, org, course, name, grader_index=None): return HttpResponse(json.dumps(CourseGradingModel.update_grader_from_json(Location(['i4x', org, course, 'course', name]), request.POST)), mimetype="application/json") - + ## NB: expect_json failed on ["key", "key2"] and json payload @login_required @ensure_csrf_cookie @@ -1363,6 +1391,7 @@ def asset_index(request, org, course, name): def edge(request): return render_to_response('university_profiles/edge.html', {}) + @login_required @expect_json def create_new_course(request): @@ -1418,6 +1447,7 @@ def create_new_course(request): return HttpResponse(json.dumps({'id': new_course.location.url()})) + def initialize_course_tabs(course): # set up the default tabs # I've added this because when we add static tabs, the LMS either expects a None for the tabs list or @@ -1435,6 +1465,7 @@ def initialize_course_tabs(course): modulestore('direct').update_metadata(course.location.url(), course.own_metadata) + @ensure_csrf_cookie @login_required def import_course(request, org, course, name): @@ -1512,6 +1543,7 @@ def import_course(request, org, course, name): course_module.location.name]) }) + @ensure_csrf_cookie @login_required def generate_export_course(request, org, course, name): @@ -1563,6 +1595,7 @@ def export_course(request, org, course, name): 'successful_import_redirect_url': '' }) + def event(request): ''' A noop to swallow the analytics call so that cms methods don't spook and poor developers looking at diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index d088d75665..24245a39d5 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -10,7 +10,7 @@ class CourseMetadata(object): ''' # __new_advanced_key__ is used by client not server; so, could argue against it being here FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start', 'end', 'enrollment_start', 'enrollment_end', 'tabs', 'graceperiod', '__new_advanced_key__'] - + @classmethod def fetch(cls, course_location): """ @@ -18,17 +18,17 @@ class CourseMetadata(object): """ if not isinstance(course_location, Location): course_location = Location(course_location) - + course = {} - + descriptor = get_modulestore(course_location).get_item(course_location) - + for k, v in descriptor.metadata.iteritems(): if k not in cls.FILTERED_LIST: course[k] = v - + return course - + @classmethod def update_from_json(cls, course_location, jsondict): """ @@ -37,7 +37,7 @@ class CourseMetadata(object): Ensures none of the fields are in the blacklist. """ descriptor = get_modulestore(course_location).get_item(course_location) - + dirty = False for k, v in jsondict.iteritems(): @@ -45,26 +45,26 @@ class CourseMetadata(object): if k not in cls.FILTERED_LIST and (k not in descriptor.metadata or descriptor.metadata[k] != v): dirty = True descriptor.metadata[k] = v - + if dirty: get_modulestore(course_location).update_metadata(course_location, descriptor.metadata) - + # Could just generate and return a course obj w/o doing any db reads, but I put the reads in as a means to confirm # it persisted correctly return cls.fetch(course_location) - + @classmethod def delete_key(cls, course_location, payload): ''' Remove the given metadata key(s) from the course. payload can be a single key or [key..] ''' descriptor = get_modulestore(course_location).get_item(course_location) - + for key in payload['deleteKeys']: if key in descriptor.metadata: del descriptor.metadata[key] - + get_modulestore(course_location).update_metadata(course_location, descriptor.metadata) - + return cls.fetch(course_location) \ No newline at end of file diff --git a/cms/envs/aws.py b/cms/envs/aws.py index a147f84531..be7816d21f 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -62,3 +62,6 @@ AWS_SECRET_ACCESS_KEY = AUTH_TOKENS["AWS_SECRET_ACCESS_KEY"] DATABASES = AUTH_TOKENS['DATABASES'] MODULESTORE = AUTH_TOKENS['MODULESTORE'] CONTENTSTORE = AUTH_TOKENS['CONTENTSTORE'] + +# Datadog for events! +DATADOG_API = AUTH_TOKENS.get("DATADOG_API") \ No newline at end of file diff --git a/cms/envs/dev.py b/cms/envs/dev.py index 3dee93a398..9164c02e3f 100644 --- a/cms/envs/dev.py +++ b/cms/envs/dev.py @@ -4,9 +4,6 @@ This config file runs the simplest dev environment""" from .common import * from logsettings import get_logger_config -import logging -import sys - DEBUG = True TEMPLATE_DEBUG = DEBUG LOGGING = get_logger_config(ENV_ROOT / "log", @@ -107,3 +104,36 @@ CACHE_TIMEOUT = 0 # Dummy secret key for dev SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' + +################################ DEBUG TOOLBAR ################################# +INSTALLED_APPS += ('debug_toolbar', 'debug_toolbar_mongo') +MIDDLEWARE_CLASSES += ('django_comment_client.utils.QueryCountDebugMiddleware', + 'debug_toolbar.middleware.DebugToolbarMiddleware',) +INTERNAL_IPS = ('127.0.0.1',) + +DEBUG_TOOLBAR_PANELS = ( + 'debug_toolbar.panels.version.VersionDebugPanel', + 'debug_toolbar.panels.timer.TimerDebugPanel', + 'debug_toolbar.panels.settings_vars.SettingsVarsDebugPanel', + 'debug_toolbar.panels.headers.HeaderDebugPanel', + 'debug_toolbar.panels.request_vars.RequestVarsDebugPanel', + 'debug_toolbar.panels.sql.SQLDebugPanel', + 'debug_toolbar.panels.signals.SignalDebugPanel', + 'debug_toolbar.panels.logger.LoggingPanel', +# This is breaking Mongo updates-- Christina is investigating. +# 'debug_toolbar_mongo.panel.MongoDebugPanel', + + # Enabling the profiler has a weird bug as of django-debug-toolbar==0.9.4 and + # Django=1.3.1/1.4 where requests to views get duplicated (your method gets + # hit twice). So you can uncomment when you need to diagnose performance + # problems, but you shouldn't leave it on. + # 'debug_toolbar.panels.profiling.ProfilingDebugPanel', + ) + +DEBUG_TOOLBAR_CONFIG = { + 'INTERCEPT_REDIRECTS': False +} + +# To see stacktraces for MongoDB queries, set this to True. +# Stacktraces slow down page loads drastically (for pages with lots of queries). +# DEBUG_TOOLBAR_MONGO_STACKTRACES = False diff --git a/cms/one_time_startup.py b/cms/one_time_startup.py new file mode 100644 index 0000000000..93428a3404 --- /dev/null +++ b/cms/one_time_startup.py @@ -0,0 +1,6 @@ +from dogapi import dog_http_api, dog_stats_api +from django.conf import settings + +if hasattr(settings, 'DATADOG_API'): + dog_http_api.api_key = settings.DATADOG_API + dog_stats_api.start(api_key=settings.DATADOG_API, statsd=True) diff --git a/cms/static/js/views/settings/advanced_view.js b/cms/static/js/views/settings/advanced_view.js index d20a21f7e7..a933bbdb9b 100644 --- a/cms/static/js/views/settings/advanced_view.js +++ b/cms/static/js/views/settings/advanced_view.js @@ -31,7 +31,8 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ // because these are outside of this.$el, they can't be in the event hash $('.save-button').on('click', this, this.saveView); $('.cancel-button').on('click', this, this.revertView); - this.model.on('error', this.handleValidationError, this); + this.listenTo(this.model, 'error', CMS.ServerError); + this.listenTo(this.model, 'invalid', this.handleValidationError); }, render: function() { // catch potential outside call before template loaded @@ -228,7 +229,7 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ var error = {}; error[oldKey] = 'You have already defined "' + newKey + '" in the manual policy definitions.'; error[newKey] = "You tried to enter a duplicate of this key."; - this.model.trigger("error", this.model, error); + this.model.trigger("invalid", this.model, error); return false; } @@ -244,7 +245,7 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ // swap to the key which the map knows about validation[oldKey] = validation[newKey]; } - this.model.trigger("error", this.model, validation); + this.model.trigger("invalid", this.model, validation); // abandon update return; } diff --git a/cms/static/js/views/settings/main_settings_view.js b/cms/static/js/views/settings/main_settings_view.js index 8f998dbf7a..9bd8feab8c 100644 --- a/cms/static/js/views/settings/main_settings_view.js +++ b/cms/static/js/views/settings/main_settings_view.js @@ -26,7 +26,8 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({ var dateIntrospect = new Date(); this.$el.find('#timezone').html("(" + dateIntrospect.getTimezone() + ")"); - this.model.on('error', this.handleValidationError, this); + this.listenTo(this.model, 'error', CMS.ServerError); + this.listenTo(this.model, 'invalid', this.handleValidationError); this.selectorToField = _.invert(this.fieldToSelectorMap); }, diff --git a/cms/static/js/views/settings/settings_grading_view.js b/cms/static/js/views/settings/settings_grading_view.js index a7c8defb43..78972f97a7 100644 --- a/cms/static/js/views/settings/settings_grading_view.js +++ b/cms/static/js/views/settings/settings_grading_view.js @@ -44,7 +44,8 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({ self.render(); } ); - this.model.on('error', this.handleValidationError, this); + this.listenTo(this.model, 'error', CMS.ServerError); + this.listenTo(this.model, 'invalid', this.handleValidationError); this.model.get('graders').on('remove', this.render, this); this.model.get('graders').on('reset', this.render, this); this.model.get('graders').on('add', this.render, this); @@ -316,7 +317,8 @@ CMS.Views.Settings.GraderView = CMS.Views.ValidatingView.extend({ 'blur :input' : "inputUnfocus" }, initialize : function() { - this.model.on('error', this.handleValidationError, this); + this.listenTo(this.model, 'error', CMS.ServerError); + this.listenTo(this.model, 'invalid', this.handleValidationError); this.selectorToField = _.invert(this.fieldToSelectorMap); this.render(); }, diff --git a/cms/static/js/views/validating_view.js b/cms/static/js/views/validating_view.js index e4928a8ebe..041e779030 100644 --- a/cms/static/js/views/validating_view.js +++ b/cms/static/js/views/validating_view.js @@ -3,7 +3,8 @@ CMS.Views.ValidatingView = Backbone.View.extend({ // decorates the fields. Needs wiring per class, but this initialization shows how // either have your init call this one or copy the contents initialize : function() { - this.model.on('error', this.handleValidationError, this); + this.listenTo(this.model, 'error', CMS.ServerError); + this.listenTo(this.model, 'invalid', this.handleValidationError); this.selectorToField = _.invert(this.fieldToSelectorMap); }, @@ -18,20 +19,11 @@ CMS.Views.ValidatingView = Backbone.View.extend({ // which may be the subjects of validation errors }, _cacheValidationErrors : [], + handleValidationError : function(model, error) { - // error triggered either by validation or server error // error is object w/ fields and error strings for (var field in error) { var ele = this.$el.find('#' + this.fieldToSelectorMap[field]); - if (ele.length === 0) { - // check if it might a server error: note a typo in the field name - // or failure to put in a map may cause this to muffle validation errors - if (_.has(error, 'error') && _.has(error, 'responseText')) { - CMS.ServerError(model, error); - return; - } - else continue; - } this._cacheValidationErrors.push(ele); if ($(ele).is('div')) { // put error on the contained inputs diff --git a/cms/templates/asset_index.html b/cms/templates/asset_index.html index 5ace98df56..a5a9144b07 100644 --- a/cms/templates/asset_index.html +++ b/cms/templates/asset_index.html @@ -28,7 +28,7 @@ {{uploadDate}}
@@ -37,3 +34,4 @@
Error:
%s' % str(err).replace('<', '<') + msg += '
%s' % traceback.format_exc().replace('<', '<') + html = msg + + # We're in non-debug mode, and possibly even in production. We want + # to avoid bricking of problem as much as possible + else: + + # Presumably, student submission has corrupted LoncapaProblem HTML. + # First, pull down all student answers + student_answers = self.lcp.student_answers + answer_ids = student_answers.keys() + + # Some inputtypes, such as dynamath, have additional "hidden" state that + # is not exposed to the student. Keep those hidden + # TODO: Use regex, e.g. 'dynamath' is suffix at end of answer_id + hidden_state_keywords = ['dynamath'] + for answer_id in answer_ids: + for hidden_state_keyword in hidden_state_keywords: + if answer_id.find(hidden_state_keyword) >= 0: + student_answers.pop(answer_id) + + # Next, generate a fresh LoncapaProblem + self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), + state=None, # Tabula rasa + seed=self.seed, system=self.system) + + # Prepend a scary warning to the student + warning = '
Error:
%s' % str(err).replace('<', '<') - msg += '
%s' % traceback.format_exc().replace('<', '<') - html = msg - else: - # We're in non-debug mode, and possibly even in production. We want - # to avoid bricking of problem as much as possible - # Presumably, student submission has corrupted LoncapaProblem HTML. - # First, pull down all student answers - student_answers = self.lcp.student_answers - answer_ids = student_answers.keys() - - # Some inputtypes, such as dynamath, have additional "hidden" state that - # is not exposed to the student. Keep those hidden - # TODO: Use regex, e.g. 'dynamath' is suffix at end of answer_id - hidden_state_keywords = ['dynamath'] - for answer_id in answer_ids: - for hidden_state_keyword in hidden_state_keywords: - if answer_id.find(hidden_state_keyword) >= 0: - student_answers.pop(answer_id) - - # Next, generate a fresh LoncapaProblem - self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), - state=None, # Tabula rasa - seed=self.seed, system=self.system) - - # Prepend a scary warning to the student - warning = '
The main goal of this exercise is to start practicing the art of slow reading.
+', '', clean_html))
except:
@@ -282,7 +282,7 @@ class OpenEndedChild(object):
"""
#This is a dev_facing_error
log.warning("Open ended child state out sync. state: %r, get: %r. %s",
- self.state, get, msg)
+ self.state, get, msg)
#This is a student_facing_error
return {'success': False,
'error': 'The problem state got out-of-sync. Please try reloading the page.'}
@@ -308,7 +308,7 @@ class OpenEndedChild(object):
@return: Boolean correct.
"""
correct = False
- if(isinstance(score, (int, long, float, complex))):
+ if (isinstance(score, (int, long, float, complex))):
score_ratio = int(score) / float(self.max_score())
correct = (score_ratio >= 0.66)
return correct
@@ -342,7 +342,8 @@ class OpenEndedChild(object):
try:
image_data.seek(0)
- success, s3_public_url = open_ended_image_submission.upload_to_s3(image_data, image_key, self.s3_interface)
+ success, s3_public_url = open_ended_image_submission.upload_to_s3(image_data, image_key,
+ self.s3_interface)
except:
log.exception("Could not upload image to S3.")
@@ -404,9 +405,9 @@ class OpenEndedChild(object):
#In this case, an image was submitted by the student, but the image could not be uploaded to S3. Likely
#a config issue (development vs deployment). For now, just treat this as a "success"
log.exception("Student AJAX post to combined open ended xmodule indicated that it contained an image, "
- "but the image was not able to be uploaded to S3. This could indicate a config"
- "issue with this deployment, but it could also indicate a problem with S3 or with the"
- "student image itself.")
+ "but the image was not able to be uploaded to S3. This could indicate a config"
+ "issue with this deployment, but it could also indicate a problem with S3 or with the"
+ "student image itself.")
overall_success = True
elif not has_file_to_upload:
#If there is no file to upload, probably the student has embedded the link in the answer text
@@ -445,7 +446,7 @@ class OpenEndedChild(object):
response = {}
#This is a student_facing_error
error_string = ("You need to peer grade {0} more in order to make another submission. "
- "You have graded {1}, and {2} are required. You have made {3} successful peer grading submissions.")
+ "You have graded {1}, and {2} are required. You have made {3} successful peer grading submissions.")
try:
response = self.peer_gs.get_data_for_location(self.location_string, student_id)
count_graded = response['count_graded']
@@ -454,16 +455,18 @@ class OpenEndedChild(object):
success = True
except:
#This is a dev_facing_error
- log.error("Could not contact external open ended graders for location {0} and student {1}".format(self.location_string,student_id))
+ log.error("Could not contact external open ended graders for location {0} and student {1}".format(
+ self.location_string, student_id))
#This is a student_facing_error
error_message = "Could not contact the graders. Please notify course staff."
return success, allowed_to_submit, error_message
- if count_graded>=count_required:
+ if count_graded >= count_required:
return success, allowed_to_submit, ""
else:
allowed_to_submit = False
#This is a student_facing_error
- error_message = error_string.format(count_required-count_graded, count_graded, count_required, student_sub_count)
+ error_message = error_string.format(count_required - count_graded, count_graded, count_required,
+ student_sub_count)
return success, allowed_to_submit, error_message
def get_eta(self):
@@ -478,7 +481,7 @@ class OpenEndedChild(object):
success = response['success']
if isinstance(success, basestring):
- success = (success.lower()=="true")
+ success = (success.lower() == "true")
if success:
eta = controller_query_service.convert_seconds_to_human_readable(response['eta'])
@@ -487,6 +490,3 @@ class OpenEndedChild(object):
eta_string = ""
return eta_string
-
-
-
diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py
index 42c54f0463..5daf1b83b5 100644
--- a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py
+++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py
@@ -14,6 +14,7 @@ class PeerGradingService(GradingService):
"""
Interface with the grading controller for peer grading
"""
+
def __init__(self, config, system):
config['system'] = system
super(PeerGradingService, self).__init__(config)
@@ -36,10 +37,11 @@ class PeerGradingService(GradingService):
def get_next_submission(self, problem_location, grader_id):
response = self.get(self.get_next_submission_url,
- {'location': problem_location, 'grader_id': grader_id})
+ {'location': problem_location, 'grader_id': grader_id})
return self.try_to_decode(self._render_rubric(response))
- def save_grade(self, location, grader_id, submission_id, score, feedback, submission_key, rubric_scores, submission_flagged):
+ def save_grade(self, location, grader_id, submission_id, score, feedback, submission_key, rubric_scores,
+ submission_flagged):
data = {'grader_id': grader_id,
'submission_id': submission_id,
'score': score,
@@ -89,6 +91,7 @@ class PeerGradingService(GradingService):
pass
return text
+
"""
This is a mock peer grading service that can be used for unit tests
without making actual service calls to the grading controller
@@ -122,7 +125,7 @@ class MockPeerGradingService(object):
'max_score': 4})
def save_calibration_essay(self, problem_location, grader_id,
- calibration_essay_id, submission_key, score,
+ calibration_essay_id, submission_key, score,
feedback, rubric_scores):
return {'success': True, 'actual_score': 2}
diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py
index f4be426667..8911e2890f 100644
--- a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py
+++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py
@@ -73,7 +73,6 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
html = system.render_template('{0}/self_assessment_prompt.html'.format(self.TEMPLATE_DIR), context)
return html
-
def handle_ajax(self, dispatch, get, system):
"""
This is called by courseware.module_render, to handle an AJAX call.
@@ -95,7 +94,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
#This is a dev_facing_error
log.error("Cannot find {0} in handlers in handle_ajax function for open_ended_module.py".format(dispatch))
#This is a dev_facing_error
- return json.dumps({'error': 'Error handling action. Please try again.', 'success' : False})
+ return json.dumps({'error': 'Error handling action. Please try again.', 'success': False})
before = self.get_progress()
d = handlers[dispatch](get, system)
@@ -159,7 +158,6 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
return system.render_template('{0}/self_assessment_hint.html'.format(self.TEMPLATE_DIR), context)
-
def save_answer(self, get, system):
"""
After the answer is submitted, show the rubric.
@@ -224,7 +222,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
try:
score = int(get['assessment'])
score_list = get.getlist('score_list[]')
- for i in xrange(0,len(score_list)):
+ for i in xrange(0, len(score_list)):
score_list[i] = int(score_list[i])
except ValueError:
#This is a dev_facing_error
@@ -268,7 +266,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
'allow_reset': self._allow_reset()}
def latest_post_assessment(self, system):
- latest_post_assessment = super(SelfAssessmentModule, self).latest_post_assessment(system)
+ latest_post_assessment = super(SelfAssessmentModule, self).latest_post_assessment(system)
try:
rubric_scores = json.loads(latest_post_assessment)
except:
@@ -305,7 +303,9 @@ class SelfAssessmentDescriptor(XmlDescriptor, EditingDescriptor):
for child in expected_children:
if len(xml_object.xpath(child)) != 1:
#This is a staff_facing_error
- raise ValueError("Self assessment definition must include exactly one '{0}' tag. Contact the learning sciences group for assistance.".format(child))
+ raise ValueError(
+ "Self assessment definition must include exactly one '{0}' tag. Contact the learning sciences group for assistance.".format(
+ child))
def parse(k):
"""Assumes that xml_object has child k"""
diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py
index 1e52dcf070..2ea8ab0db5 100644
--- a/common/lib/xmodule/xmodule/peer_grading_module.py
+++ b/common/lib/xmodule/xmodule/peer_grading_module.py
@@ -5,7 +5,7 @@ from lxml import etree
from datetime import datetime
from pkg_resources import resource_string
-from .capa_module import ComplexEncoder
+from .capa_module import ComplexEncoder
from .editing_module import EditingDescriptor
from .stringify import stringify_children
from .x_module import XModule
@@ -34,7 +34,7 @@ class PeerGradingModule(XModule):
resource_string(__name__, 'js/src/peergrading/peer_grading_problem.coffee'),
resource_string(__name__, 'js/src/collapsible.coffee'),
resource_string(__name__, 'js/src/javascript_loader.coffee'),
- ]}
+ ]}
js_module_name = "PeerGrading"
css = {'scss': [resource_string(__name__, 'css/combinedopenended/display.scss')]}
@@ -42,7 +42,7 @@ class PeerGradingModule(XModule):
def __init__(self, system, location, definition, descriptor,
instance_state=None, shared_state=None, **kwargs):
XModule.__init__(self, system, location, definition, descriptor,
- instance_state, shared_state, **kwargs)
+ instance_state, shared_state, **kwargs)
# Load instance state
if instance_state is not None:
@@ -53,12 +53,11 @@ class PeerGradingModule(XModule):
#We need to set the location here so the child modules can use it
system.set('location', location)
self.system = system
- if(self.system.open_ended_grading_interface):
+ if (self.system.open_ended_grading_interface):
self.peer_gs = PeerGradingService(self.system.open_ended_grading_interface, self.system)
else:
self.peer_gs = MockPeerGradingService()
-
self.use_for_single_location = self.metadata.get('use_for_single_location', USE_FOR_SINGLE_LOCATION)
if isinstance(self.use_for_single_location, basestring):
self.use_for_single_location = (self.use_for_single_location in TRUE_DICT)
@@ -83,14 +82,13 @@ class PeerGradingModule(XModule):
grace_period_string = self.metadata.get('graceperiod', None)
try:
- self.timeinfo = TimeInfo(display_due_date_string, grace_period_string)
+ self.timeinfo = TimeInfo(display_due_date_string, grace_period_string)
except:
log.error("Error parsing due date information in location {0}".format(location))
raise
self.display_due_date = self.timeinfo.display_due_date
-
self.ajax_url = self.system.ajax_url
if not self.ajax_url.endswith("/"):
self.ajax_url = self.ajax_url + "/"
@@ -148,13 +146,13 @@ class PeerGradingModule(XModule):
'save_grade': self.save_grade,
'save_calibration_essay': self.save_calibration_essay,
'problem': self.peer_grading_problem,
- }
+ }
if dispatch not in handlers:
#This is a dev_facing_error
log.error("Cannot find {0} in handlers in handle_ajax function for open_ended_module.py".format(dispatch))
#This is a dev_facing_error
- return json.dumps({'error': 'Error handling action. Please try again.', 'success' : False})
+ return json.dumps({'error': 'Error handling action. Please try again.', 'success': False})
d = handlers[dispatch](get)
@@ -191,9 +189,10 @@ class PeerGradingModule(XModule):
except:
success, response = self.query_data_for_location()
if not success:
- log.exception("No instance data found and could not get data from controller for loc {0} student {1}".format(
- self.system.location.url(), self.system.anonymous_student_id
- ))
+ log.exception(
+ "No instance data found and could not get data from controller for loc {0} student {1}".format(
+ self.system.location.url(), self.system.anonymous_student_id
+ ))
return None
count_graded = response['count_graded']
count_required = response['count_required']
@@ -204,7 +203,7 @@ class PeerGradingModule(XModule):
score_dict = {
'score': int(count_graded >= count_required),
'total': self.max_grade,
- }
+ }
return score_dict
@@ -253,7 +252,7 @@ class PeerGradingModule(XModule):
.format(self.peer_gs.url, location, grader_id))
#This is a student_facing_error
return {'success': False,
- 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR}
+ 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR}
def save_grade(self, get):
"""
@@ -271,7 +270,8 @@ class PeerGradingModule(XModule):
error: if there was an error in the submission, this is the error message
"""
- required = set(['location', 'submission_id', 'submission_key', 'score', 'feedback', 'rubric_scores[]', 'submission_flagged'])
+ required = set(['location', 'submission_id', 'submission_key', 'score', 'feedback', 'rubric_scores[]',
+ 'submission_flagged'])
success, message = self._check_required(get, required)
if not success:
return self._err_response(message)
@@ -287,14 +287,14 @@ class PeerGradingModule(XModule):
try:
response = self.peer_gs.save_grade(location, grader_id, submission_id,
- score, feedback, submission_key, rubric_scores, submission_flagged)
+ score, feedback, submission_key, rubric_scores, submission_flagged)
return response
except GradingServiceError:
#This is a dev_facing_error
log.exception("""Error saving grade to open ended grading service. server url: {0}, location: {1}, submission_id:{2},
submission_key: {3}, score: {4}"""
.format(self.peer_gs.url,
- location, submission_id, submission_key, score)
+ location, submission_id, submission_key, score)
)
#This is a student_facing_error
return {
@@ -382,7 +382,7 @@ class PeerGradingModule(XModule):
.format(self.peer_gs.url, location))
#This is a student_facing_error
return {'success': False,
- 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR}
+ 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR}
# if we can't parse the rubric into HTML,
except etree.XMLSyntaxError:
#This is a dev_facing_error
@@ -390,7 +390,7 @@ class PeerGradingModule(XModule):
.format(rubric))
#This is a student_facing_error
return {'success': False,
- 'error': 'Error displaying submission. Please notify course staff.'}
+ 'error': 'Error displaying submission. Please notify course staff.'}
def save_calibration_essay(self, get):
@@ -426,11 +426,13 @@ class PeerGradingModule(XModule):
try:
response = self.peer_gs.save_calibration_essay(location, grader_id, calibration_essay_id,
- submission_key, score, feedback, rubric_scores)
+ submission_key, score, feedback, rubric_scores)
return response
except GradingServiceError:
#This is a dev_facing_error
- log.exception("Error saving calibration grade, location: {0}, submission_id: {1}, submission_key: {2}, grader_id: {3}".format(location, submission_id, submission_key, grader_id))
+ log.exception(
+ "Error saving calibration grade, location: {0}, submission_id: {1}, submission_key: {2}, grader_id: {3}".format(
+ location, submission_id, submission_key, grader_id))
#This is a student_facing_error
return self._err_response('There was an error saving your score. Please notify course staff.')
@@ -440,7 +442,7 @@ class PeerGradingModule(XModule):
'''
html = self.system.render_template('peer_grading/peer_grading_closed.html', {
'use_for_single_location': self.use_for_single_location
- })
+ })
return html
@@ -503,12 +505,11 @@ class PeerGradingModule(XModule):
problem['closed'] = True
else:
problem['closed'] = False
- else:
- # if we can't find the due date, assume that it doesn't have one
+ else:
+ # if we can't find the due date, assume that it doesn't have one
problem['due'] = None
problem['closed'] = False
-
ajax_url = self.ajax_url
html = self.system.render_template('peer_grading/peer_grading.html', {
'course_id': self.system.course_id,
@@ -519,7 +520,7 @@ class PeerGradingModule(XModule):
# Checked above
'staff_access': False,
'use_single_location': self.use_for_single_location,
- })
+ })
return html
@@ -531,7 +532,8 @@ class PeerGradingModule(XModule):
if not self.use_for_single_location:
#This is an error case, because it must be set to use a single location to be called without get parameters
#This is a dev_facing_error
- log.error("Peer grading problem in peer_grading_module called with no get parameters, but use_for_single_location is False.")
+ log.error(
+ "Peer grading problem in peer_grading_module called with no get parameters, but use_for_single_location is False.")
return {'html': "", 'success': False}
problem_location = self.link_to_location
@@ -547,7 +549,7 @@ class PeerGradingModule(XModule):
# Checked above
'staff_access': False,
'use_single_location': self.use_for_single_location,
- })
+ })
return {'html': html, 'success': True}
@@ -560,7 +562,7 @@ class PeerGradingModule(XModule):
state = {
'student_data_for_location': self.student_data_for_location,
- }
+ }
return json.dumps(state)
@@ -596,7 +598,9 @@ class PeerGradingDescriptor(XmlDescriptor, EditingDescriptor):
for child in expected_children:
if len(xml_object.xpath(child)) == 0:
#This is a staff_facing_error
- raise ValueError("Peer grading definition must include at least one '{0}' tag. Contact the learning sciences group for assistance.".format(child))
+ raise ValueError(
+ "Peer grading definition must include at least one '{0}' tag. Contact the learning sciences group for assistance.".format(
+ child))
def parse_task(k):
"""Assumes that xml_object has child k"""
diff --git a/common/lib/xmodule/xmodule/templates/annotatable/default.yaml b/common/lib/xmodule/xmodule/templates/annotatable/default.yaml
new file mode 100644
index 0000000000..31dd489fb4
--- /dev/null
+++ b/common/lib/xmodule/xmodule/templates/annotatable/default.yaml
@@ -0,0 +1,20 @@
+---
+metadata:
+ display_name: 'Annotation'
+data: |
+ Enter your (optional) instructions for the exercise in HTML format. Annotations are specified by an Add your HTML with annotation spans here. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla facilisi.
+ <annotation> tag which may may have the following attributes:
+
+ title (optional). Title of the annotation. Defaults to Commentary if omitted.body (required). Text of the annotation.problem (optional). Numeric index of the problem associated with this annotation. This is a zero-based index, so the first problem on the page would have problem="0".highlight (optional). Possible values: yellow, red, orange, green, blue, or purple. Defaults to yellow if this attribute is omitted.