diff --git a/.gitignore b/.gitignore index 493df5a7fd..b13a128a63 100644 --- a/.gitignore +++ b/.gitignore @@ -28,4 +28,5 @@ nosetests.xml cover_html/ .idea/ .redcar/ -chromedriver.log \ No newline at end of file +chromedriver.log +ghostdriver.log diff --git a/.pylintrc b/.pylintrc index ce2f2e3b87..4357885133 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 @@ -43,7 +43,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 +97,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)$ # Regular expression which should only match correct class names class-rgx=[A-Z_][a-zA-Z0-9]+$ 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 c0ab9ec60e..99ef1169b1 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'}), + "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']) @@ -264,32 +292,32 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertContains(resp, '/c4x/edX/full/asset/handouts_schematic_tutorial.pdf') def test_export_course_with_unknown_metadata(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()) - course = ms.get_item(location) + 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 - ms.update_metadata(location, course.metadata) + module_store.update_metadata(location, course.metadata) print 'Exporting to tempdir = {0}'.format(root_dir) # export out to a tempdir - bExported = False + exported = False try: - export_to_xml(ms, cs, location, root_dir, 'test_export') - bExported = True + export_to_xml(module_store, content_store, location, root_dir, 'test_export') + exported = True except Exception: pass - self.assertTrue(bExported) + self.assertTrue(exported) class ContentStoreTest(ModuleStoreTestCase): """ @@ -428,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', @@ -450,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 @@ -464,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: @@ -478,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) @@ -499,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']) @@ -512,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 @@ -529,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 464342f3e1..f57299596c 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) @@ -115,6 +118,7 @@ def ux_alerts(request): # ==== Views for any logged-in user ================================== + @login_required @ensure_csrf_cookie def index(request): @@ -148,6 +152,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 @@ -396,6 +401,7 @@ def preview_component(request, location): 'editor': wrap_xmodule(component.get_html, component, 'xmodule_edit.html')(), }) + @expect_json @login_required @ensure_csrf_cookie @@ -639,6 +645,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 + + 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() @@ -712,6 +729,7 @@ def create_draft(request): return HttpResponse() + @login_required @expect_json def publish_draft(request): @@ -741,6 +759,7 @@ def unpublish_unit(request): return HttpResponse() + @login_required @expect_json def clone_item(request): @@ -771,8 +790,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 @@ -834,6 +852,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 ''' @@ -866,6 +885,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 @@ -898,6 +918,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 @@ -929,6 +950,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): @@ -1032,6 +1054,7 @@ def edit_tabs(request, org, course, coursename): 'components': components }) + def not_found(request): return render_to_response('error.html', {'error': '404'}) @@ -1067,6 +1090,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 @@ -1164,6 +1188,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): @@ -1187,6 +1212,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): @@ -1210,6 +1236,7 @@ def course_config_advanced_page(request, org, course, name): 'advanced_dict' : json.dumps(CourseMetadata.fetch(location)), }) + @expect_json @login_required @ensure_csrf_cookie @@ -1241,6 +1268,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 @@ -1275,7 +1303,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 @@ -1366,6 +1394,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): @@ -1421,6 +1450,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 @@ -1438,6 +1468,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): @@ -1515,6 +1546,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): @@ -1566,6 +1598,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/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 1aaa7d0c1e..9328f843ad 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/urls.py b/cms/urls.py index 7a384b3f20..92963f2271 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -1,5 +1,6 @@ from django.conf import settings from django.conf.urls import patterns, include, url +from . import one_time_startup # Uncomment the next two lines to enable the admin: # from django.contrib import admin diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py index f0234ec71a..c362ed4e89 100644 --- a/common/djangoapps/course_groups/cohorts.py +++ b/common/djangoapps/course_groups/cohorts.py @@ -65,23 +65,23 @@ def is_commentable_cohorted(course_id, commentable_id): ans)) return ans - + def get_cohorted_commentables(course_id): """ Given a course_id return a list of strings representing cohorted commentables """ course = courses.get_course_by_id(course_id) - + if not course.is_cohorted: # this is the easy case :) ans = [] - else: + else: ans = course.cohorted_discussions return ans - - + + def get_cohort(user, course_id): """ Given a django User and a course_id, return the user's cohort in that @@ -120,7 +120,8 @@ def get_cohort(user, course_id): return None choices = course.auto_cohort_groups - if len(choices) == 0: + n = len(choices) + if n == 0: # Nowhere to put user log.warning("Course %s is auto-cohorted, but there are no" " auto_cohort_groups specified", @@ -128,12 +129,19 @@ def get_cohort(user, course_id): return None # Put user in a random group, creating it if needed - group_name = random.choice(choices) + choice = random.randrange(0, n) + group_name = choices[choice] + + # Victor: we are seeing very strange behavior on prod, where almost all users + # end up in the same group. Log at INFO to try to figure out what's going on. + log.info("DEBUG: adding user {0} to cohort {1}. choice={2}".format( + user, group_name,choice)) + group, created = CourseUserGroup.objects.get_or_create( course_id=course_id, group_type=CourseUserGroup.COHORT, name=group_name) - + user.course_groups.add(group) return group diff --git a/common/djangoapps/course_groups/tests/tests.py b/common/djangoapps/course_groups/tests/tests.py index efed39d536..88d9c1f508 100644 --- a/common/djangoapps/course_groups/tests/tests.py +++ b/common/djangoapps/course_groups/tests/tests.py @@ -6,7 +6,7 @@ from django.test.utils import override_settings from course_groups.models import CourseUserGroup from course_groups.cohorts import (get_cohort, get_course_cohorts, - is_commentable_cohorted) + is_commentable_cohorted, get_cohort_by_name) from xmodule.modulestore.django import modulestore, _MODULESTORES @@ -168,7 +168,7 @@ class TestCohorts(django.test.TestCase): self.assertEquals(get_cohort(user3, course.id), None, "No groups->no auto-cohorting") - + # Now make it different self.config_course_cohorts(course, [], cohorted=True, auto_cohort=True, @@ -180,6 +180,37 @@ class TestCohorts(django.test.TestCase): "user2 should still be in originally placed cohort") + def test_auto_cohorting_randomization(self): + """ + Make sure get_cohort() randomizes properly. + """ + course = modulestore().get_course("edX/toy/2012_Fall") + self.assertEqual(course.id, "edX/toy/2012_Fall") + self.assertFalse(course.is_cohorted) + + groups = ["group_{0}".format(n) for n in range(5)] + self.config_course_cohorts(course, [], cohorted=True, + auto_cohort=True, + auto_cohort_groups=groups) + + # Assign 100 users to cohorts + for i in range(100): + user = User.objects.create(username="test_{0}".format(i), + email="a@b{0}.com".format(i)) + get_cohort(user, course.id) + + # Now make sure that the assignment was at least vaguely random: + # each cohort should have at least 1, and fewer than 50 students. + # (with 5 groups, probability of 0 users in any group is about + # .8**100= 2.0e-10) + for cohort_name in groups: + cohort = get_cohort_by_name(course.id, cohort_name) + num_users = cohort.users.count() + self.assertGreater(num_users, 1) + self.assertLess(num_users, 50) + + + def test_get_course_cohorts(self): course1_id = 'a/b/c' course2_id = 'e/f/g' diff --git a/common/djangoapps/heartbeat/views.py b/common/djangoapps/heartbeat/views.py index 956504407b..d7c3a32192 100644 --- a/common/djangoapps/heartbeat/views.py +++ b/common/djangoapps/heartbeat/views.py @@ -2,8 +2,9 @@ import json from datetime import datetime from django.http import HttpResponse from xmodule.modulestore.django import modulestore +from dogapi import dog_stats_api - +@dog_stats_api.timed('edxapp.heartbeat') def heartbeat(request): """ Simple view that a loadbalancer can check to verify that the app is up diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index fb1f48d143..b73a658c5f 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -84,12 +84,19 @@ def replace_static_urls(text, data_directory, course_namespace=None): if rest.endswith('?raw'): return original - # course_namespace is not None, then use studio style urls - if course_namespace is not None and not isinstance(modulestore(), XMLModuleStore): - url = StaticContent.convert_legacy_static_url(rest, course_namespace) # In debug mode, if we can find the url as is, - elif settings.DEBUG and finders.find(rest, True): + if settings.DEBUG and finders.find(rest, True): return original + # if we're running with a MongoBacked store course_namespace is not None, then use studio style urls + elif course_namespace is not None and not isinstance(modulestore(), XMLModuleStore): + # first look in the static file pipeline and see if we are trying to reference + # a piece of static content which is in the mitx repo (e.g. JS associated with an xmodule) + if staticfiles_storage.exists(rest): + url = staticfiles_storage.url(rest) + else: + # if not, then assume it's courseware specific content and then look in the + # Mongo-backed database + url = StaticContent.convert_legacy_static_url(rest, course_namespace) # Otherwise, look the file up in staticfiles_storage, and append the data directory if needed else: course_path = "/".join((data_directory, rest)) diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index 5eded6484a..75716c7443 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -10,6 +10,7 @@ import paramiko import boto dog_http_api.api_key = settings.DATADOG_API +dog_stats_api.start(api_key=settings.DATADOG_API, statsd=True) class Command(BaseCommand): diff --git a/common/djangoapps/terrain/browser.py b/common/djangoapps/terrain/browser.py index 8c2a8ba7a5..0881d86124 100644 --- a/common/djangoapps/terrain/browser.py +++ b/common/djangoapps/terrain/browser.py @@ -13,6 +13,7 @@ from django.core.management import call_command def initial_setup(server): # Launch the browser app (choose one of these below) world.browser = Browser('chrome') + # world.browser = Browser('phantomjs') # world.browser = Browser('firefox') diff --git a/common/lib/capa/capa/calc.py b/common/lib/capa/capa/calc.py index 0f062d17d5..c3fe6b656b 100644 --- a/common/lib/capa/capa/calc.py +++ b/common/lib/capa/capa/calc.py @@ -183,7 +183,7 @@ def evaluator(variables, functions, string, cs=False): # 0.33k or -17 number = (Optional(minus | plus) + inner_number - + Optional(CaselessLiteral("E") + Optional("-") + number_part) + + Optional(CaselessLiteral("E") + Optional((plus | minus)) + number_part) + Optional(number_suffix)) number = number.setParseAction(number_parse_action) # Convert to number diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 14c590a660..8b32686985 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -29,6 +29,7 @@ import sys from lxml import etree from xml.sax.saxutils import unescape +from copy import deepcopy import chem import chem.chemcalc @@ -497,11 +498,10 @@ class LoncapaProblem(object): Used by get_html. ''' - if (problemtree.tag == 'script' and problemtree.get('type') and 'javascript' in problemtree.get('type')): # leave javascript intact. - return problemtree + return deepcopy(problemtree) if problemtree.tag in html_problem_semantics: return diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index f246b406d5..ea56863a2f 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -111,15 +111,14 @@ class CorrectMap(object): return None def get_npoints(self, answer_id): - """ Return the number of points for an answer: - If the answer is correct, return the assigned - number of points (default: 1 point) - Otherwise, return 0 points """ - if self.is_correct(answer_id): - npoints = self.get_property(answer_id, 'npoints') - return npoints if npoints is not None else 1 - else: - return 0 + """Return the number of points for an answer, used for partial credit.""" + npoints = self.get_property(answer_id, 'npoints') + if npoints is not None: + return npoints + elif self.is_correct(answer_id): + return 1 + # if not correct and no points have been assigned, return 0 + return 0 def set_property(self, answer_id, property, value): if answer_id in self.cmap: diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index f614743e67..9781f10ae6 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -366,6 +366,12 @@ class ChoiceGroup(InputTypeBase): self.choices = self.extract_choices(self.xml) + @classmethod + def get_attributes(cls): + return [Attribute("show_correctness", "always"), + Attribute("submitted_message", "Answer received.")] + + def _extra_context(self): return {'input_type': self.html_input_type, 'choices': self.choices, diff --git a/common/lib/capa/capa/templates/choicegroup.html b/common/lib/capa/capa/templates/choicegroup.html index b9d8164321..e1ff40b6a1 100644 --- a/common/lib/capa/capa/templates/choicegroup.html +++ b/common/lib/capa/capa/templates/choicegroup.html @@ -1,37 +1,47 @@
diff --git a/common/lib/capa/capa/tests/test_correctmap.py b/common/lib/capa/capa/tests/test_correctmap.py index 23734467bb..270ba4d849 100644 --- a/common/lib/capa/capa/tests/test_correctmap.py +++ b/common/lib/capa/capa/tests/test_correctmap.py @@ -91,12 +91,12 @@ class CorrectMapTest(unittest.TestCase): npoints=0) # Assert that we get the expected points - # If points assigned and correct --> npoints + # If points assigned --> npoints # If no points assigned and correct --> 1 point - # Otherwise --> 0 points + # If no points assigned and incorrect --> 0 points self.assertEqual(self.cmap.get_npoints('1_2_1'), 5) self.assertEqual(self.cmap.get_npoints('2_2_1'), 1) - self.assertEqual(self.cmap.get_npoints('3_2_1'), 0) + self.assertEqual(self.cmap.get_npoints('3_2_1'), 5) self.assertEqual(self.cmap.get_npoints('4_2_1'), 0) self.assertEqual(self.cmap.get_npoints('5_2_1'), 0) diff --git a/common/lib/capa/capa/tests/test_html_render.py b/common/lib/capa/capa/tests/test_html_render.py index ca2a3c2e2c..6c74d06ef4 100644 --- a/common/lib/capa/capa/tests/test_html_render.py +++ b/common/lib/capa/capa/tests/test_html_render.py @@ -3,6 +3,7 @@ from lxml import etree import os import textwrap import json + import mock from capa.capa_problem import LoncapaProblem @@ -49,6 +50,8 @@ class CapaHtmlRenderTest(unittest.TestCase): self.assertEqual(test_element.text, "Test include") + + def test_process_outtext(self): # Generate some XML with', '', 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/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py
index 6330511fc5..cb77921957 100644
--- a/common/lib/xmodule/xmodule/tests/test_capa_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py
@@ -11,6 +11,8 @@ from xmodule.capa_module import CapaModule
from xmodule.modulestore import Location
from lxml import etree
+from django.http import QueryDict
+
from . import test_system
@@ -326,14 +328,18 @@ class CapaModuleTest(unittest.TestCase):
def test_parse_get_params(self):
+ # We have to set up Django settings in order to use QueryDict
+ from django.conf import settings
+ settings.configure()
+
# Valid GET param dict
- valid_get_dict = {'input_1': 'test',
- 'input_1_2': 'test',
- 'input_1_2_3': 'test',
- 'input_[]_3': 'test',
- 'input_4': None,
- 'input_5': [],
- 'input_6': 5}
+ valid_get_dict = self._querydict_from_dict({'input_1': 'test',
+ 'input_1_2': 'test',
+ 'input_1_2_3': 'test',
+ 'input_[]_3': 'test',
+ 'input_4': None,
+ 'input_5': [],
+ 'input_6': 5})
result = CapaModule.make_dict_of_responses(valid_get_dict)
@@ -347,20 +353,19 @@ class CapaModuleTest(unittest.TestCase):
# Valid GET param dict with list keys
- valid_get_dict = {'input_2[]': ['test1', 'test2']}
+ valid_get_dict = self._querydict_from_dict({'input_2[]': ['test1', 'test2']})
result = CapaModule.make_dict_of_responses(valid_get_dict)
self.assertTrue('2' in result)
- self.assertEqual(valid_get_dict['input_2[]'], result['2'])
+ self.assertEqual(['test1','test2'], result['2'])
# If we use [] at the end of a key name, we should always
# get a list, even if there's just one value
- valid_get_dict = {'input_1[]': 'test'}
+ valid_get_dict = self._querydict_from_dict({'input_1[]': 'test'})
result = CapaModule.make_dict_of_responses(valid_get_dict)
self.assertEqual(result['1'], ['test'])
-
# If we have no underscores in the name, then the key is invalid
- invalid_get_dict = {'input': 'test'}
+ invalid_get_dict = self._querydict_from_dict({'input': 'test'})
with self.assertRaises(ValueError):
result = CapaModule.make_dict_of_responses(invalid_get_dict)
@@ -368,11 +373,32 @@ class CapaModuleTest(unittest.TestCase):
# Two equivalent names (one list, one non-list)
# One of the values would overwrite the other, so detect this
# and raise an exception
- invalid_get_dict = {'input_1[]': 'test 1',
- 'input_1': 'test 2' }
+ invalid_get_dict = self._querydict_from_dict({'input_1[]': 'test 1',
+ 'input_1': 'test 2' })
with self.assertRaises(ValueError):
result = CapaModule.make_dict_of_responses(invalid_get_dict)
+ def _querydict_from_dict(self, param_dict):
+ """ Create a Django QueryDict from a Python dictionary """
+
+ # QueryDict objects are immutable by default, so we make
+ # a copy that we can update.
+ querydict = QueryDict('')
+ copyDict = querydict.copy()
+
+ for (key, val) in param_dict.items():
+
+ # QueryDicts handle lists differently from ordinary values,
+ # so we have to specifically tell the QueryDict that
+ # this is a list
+ if type(val) is list:
+ copyDict.setlist(key, val)
+ else:
+ copyDict[key] = val
+
+ return copyDict
+
+
def test_check_problem_correct(self):
module = CapaFactory.create(attempts=1)
diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py
index a524ac2fd9..8a14e03ded 100644
--- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py
+++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py
@@ -14,6 +14,7 @@ from datetime import datetime
from . import test_system
import test_util_open_ended
+
"""
Tests for the various pieces of the CombinedOpenEndedGrading system
@@ -39,41 +40,37 @@ class OpenEndedChildTest(unittest.TestCase):
max_score = 1
static_data = {
- 'max_attempts': 20,
- 'prompt': prompt,
- 'rubric': rubric,
- 'max_score': max_score,
- 'display_name': 'Name',
- 'accept_file_upload': False,
- 'close_date': None,
- 's3_interface' : "",
- 'open_ended_grading_interface' : {},
- 'skip_basic_checks' : False,
- }
+ 'max_attempts': 20,
+ 'prompt': prompt,
+ 'rubric': rubric,
+ 'max_score': max_score,
+ 'display_name': 'Name',
+ 'accept_file_upload': False,
+ 'close_date': None,
+ 's3_interface': "",
+ 'open_ended_grading_interface': {},
+ 'skip_basic_checks': False,
+ }
definition = Mock()
descriptor = Mock()
def setUp(self):
self.test_system = test_system()
self.openendedchild = OpenEndedChild(self.test_system, self.location,
- self.definition, self.descriptor, self.static_data, self.metadata)
-
+ self.definition, self.descriptor, self.static_data, self.metadata)
def test_latest_answer_empty(self):
answer = self.openendedchild.latest_answer()
self.assertEqual(answer, "")
-
def test_latest_score_empty(self):
answer = self.openendedchild.latest_score()
self.assertEqual(answer, None)
-
def test_latest_post_assessment_empty(self):
answer = self.openendedchild.latest_post_assessment(self.test_system)
self.assertEqual(answer, "")
-
def test_new_history_entry(self):
new_answer = "New Answer"
self.openendedchild.new_history_entry(new_answer)
@@ -99,7 +96,6 @@ class OpenEndedChildTest(unittest.TestCase):
score = self.openendedchild.latest_score()
self.assertEqual(score, 4)
-
def test_record_latest_post_assessment(self):
new_answer = "New Answer"
self.openendedchild.new_history_entry(new_answer)
@@ -107,7 +103,7 @@ class OpenEndedChildTest(unittest.TestCase):
post_assessment = "Post assessment"
self.openendedchild.record_latest_post_assessment(post_assessment)
self.assertEqual(post_assessment,
- self.openendedchild.latest_post_assessment(self.test_system))
+ self.openendedchild.latest_post_assessment(self.test_system))
def test_get_score(self):
new_answer = "New Answer"
@@ -124,24 +120,22 @@ class OpenEndedChildTest(unittest.TestCase):
self.assertEqual(score['score'], new_score)
self.assertEqual(score['total'], self.static_data['max_score'])
-
def test_reset(self):
self.openendedchild.reset(self.test_system)
state = json.loads(self.openendedchild.get_instance_state())
self.assertEqual(state['state'], OpenEndedChild.INITIAL)
-
def test_is_last_response_correct(self):
new_answer = "New Answer"
self.openendedchild.new_history_entry(new_answer)
self.openendedchild.record_latest_score(self.static_data['max_score'])
self.assertEqual(self.openendedchild.is_last_response_correct(),
- 'correct')
+ 'correct')
self.openendedchild.new_history_entry(new_answer)
self.openendedchild.record_latest_score(0)
self.assertEqual(self.openendedchild.is_last_response_correct(),
- 'incorrect')
+ 'incorrect')
class OpenEndedModuleTest(unittest.TestCase):
@@ -159,18 +153,18 @@ class OpenEndedModuleTest(unittest.TestCase):
max_score = 4
static_data = {
- 'max_attempts': 20,
- 'prompt': prompt,
- 'rubric': rubric,
- 'max_score': max_score,
- 'display_name': 'Name',
- 'accept_file_upload': False,
- 'rewrite_content_links' : "",
- 'close_date': None,
- 's3_interface' : test_util_open_ended.S3_INTERFACE,
- 'open_ended_grading_interface' : test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE,
- 'skip_basic_checks' : False,
- }
+ 'max_attempts': 20,
+ 'prompt': prompt,
+ 'rubric': rubric,
+ 'max_score': max_score,
+ 'display_name': 'Name',
+ 'accept_file_upload': False,
+ 'rewrite_content_links': "",
+ 'close_date': None,
+ 's3_interface': test_util_open_ended.S3_INTERFACE,
+ 'open_ended_grading_interface': test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE,
+ 'skip_basic_checks': False,
+ }
oeparam = etree.XML('''