From 54ee1607b8c73e9d48e65490511b1221853d9500 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 23 Jan 2013 11:22:37 -0500 Subject: [PATCH 01/60] add unit test to test DraftModuleStore.get_items() fix --- lms/djangoapps/courseware/tests/tests.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index bf8a65126d..fad8ac6fef 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -64,6 +64,21 @@ def mongo_store_config(data_dir): } } +def draft_mongo_store_config(data_dir): + return { + 'default': { + 'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore', + 'OPTIONS': { + 'default_class': 'xmodule.raw_module.RawDescriptor', + 'host': 'localhost', + 'db': 'test_xmodule', + 'collection': 'modulestore', + 'fs_root': data_dir, + 'render_template': 'mitxmako.shortcuts.render_to_string', + } + } +} + def xml_store_config(data_dir): return { 'default': { @@ -78,6 +93,7 @@ def xml_store_config(data_dir): TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT TEST_DATA_XML_MODULESTORE = xml_store_config(TEST_DATA_DIR) TEST_DATA_MONGO_MODULESTORE = mongo_store_config(TEST_DATA_DIR) +TEST_DATA_DRAFT_MONGO_MODULESTORE = draft_mongo_store_config(TEST_DATA_DIR) class ActivateLoginTestCase(TestCase): '''Check that we can activate and log in''' @@ -423,6 +439,14 @@ class TestNavigation(PageLoader): kwargs={'course_id': self.toy.id, 'chapter': 'secret:magic'})) +@override_settings(MODULESTORE=TEST_DATA_DRAFT_MONGO_MODULESTORE) +class TestDraftModuleStore(TestCase): + def test_get_items_with_course_items(self): + store = modulestore() + # fix was to allow get_items() to take the course_id parameter + store.get_items(Location(None, None, 'vertical', None, None), course_id='abc', depth=0) + + @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) class TestViewAuth(PageLoader): """Check that view authentication works properly""" From dc3dd41aa77d50d180c451e52b2761fc42c91c8a Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 24 Jan 2013 08:58:24 -0500 Subject: [PATCH 02/60] Update tests to cover the peer grading service --- lms/djangoapps/open_ended_grading/tests.py | 83 +++++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 0c4376a44b..bd2b3c6695 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -6,6 +6,7 @@ django-admin.py test --settings=lms.envs.test --pythonpath=. lms/djangoapps/open from django.test import TestCase from open_ended_grading import staff_grading_service +from open_ended_grading import peer_grading_service from django.core.urlresolvers import reverse from django.contrib.auth.models import Group @@ -19,7 +20,6 @@ import json from override_settings import override_settings -_mock_service = staff_grading_service.MockStaffGradingService() @override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE) class TestStaffGradingService(ct.PageLoader): @@ -110,3 +110,84 @@ class TestStaffGradingService(ct.PageLoader): d = json.loads(r.content) self.assertTrue(d['success'], str(d)) self.assertIsNotNone(d['problem_list']) + + +@override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE) +class TestPeerGradingService(ct.PageLoader): + ''' + Check that staff grading service proxy works. Basically just checking the + access control and error handling logic -- all the actual work is on the + backend. + ''' + def setUp(self): + xmodule.modulestore.django._MODULESTORES = {} + + self.student = 'view@test.com' + self.instructor = 'view2@test.com' + self.password = 'foo' + self.location = 'TestLocation' + self.create_account('u1', self.student, self.password) + self.create_account('u2', self.instructor, self.password) + self.activate_user(self.student) + self.activate_user(self.instructor) + + self.course_id = "edX/toy/2012_Fall" + self.toy = modulestore().get_course(self.course_id) + + self.mock_service = peer_grading_service.peer_grading_service() + + self.logout() + + def test_get_next_submission_success(self): + self.login(self.student, self.password) + + url = reverse('peer_grading_get_next_submission', kwargs={'course_id': self.course_id}) + data = {'location': self.location} + + r = self.check_for_post_code(200, url, data) + d = json.loads(r.content) + self.assertTrue(d['success']) + self.assertIsNotNone(d['submission_id']) + self.assertIsNotNone(d['prompt']) + self.assertIsNotNone(d['submission_key']) + self.assertIsNotNone(d['max_score']) + + def test_get_next_submission_missing_location(self): + self.login(self.student, self.password) + url = reverse('peer_grading_get_next_submission', kwargs={'course_id': self.course_id}) + data = {} + r = self.check_for_post_code(200, url, data) + d = json.loads(r.content) + self.assertFalse(d['success']) + self.assertEqual(d['error'], "Missing required keys: location") + + def test_save_grade_success(self): + self.login(self.student, self.password) + url = reverse('peer_grading_save_grade', kwargs={'course_id': self.course_id}) + data = {'location': self.location, + 'submission_id': '1', + 'submission_key': 'fake key', + 'score': '2', + 'feedback': 'This is feedback'} + r = self.check_for_post_code(200, url, data) + d = json.loads(r.content) + self.assertTrue(d['success']) + + def test_save_grade_missing_keys(self): + self.login(self.student, self.password) + url = reverse('peer_grading_save_grade', kwargs={'course_id': self.course_id}) + data = {} + r = self.check_for_post_code(200, url, data) + d = json.loads(r.content) + self.assertFalse(d['success']) + self.assertTrue(d['error'].find('Missing required keys:') > -1) + + def test_is_calibrated(self): + self.login(self.student, self.password) + url = reverse('peer_grading_is_student_calibrated', kwargs={'course_id': self.course_id}) + data = {'location': self.location} + r = self.check_for_post_code(200, url, data) + d = json.loads(r.content) + self.assertTrue(d['success']) + self.assertTrue('calibrated' in d) + From 210cdf870eb5e2d8d27c10c5676cf87968fbc97f Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 24 Jan 2013 09:41:24 -0500 Subject: [PATCH 03/60] Updates to the mock peer grading service and new tests. --- .../peer_grading_service.py | 15 ++++- lms/djangoapps/open_ended_grading/tests.py | 62 ++++++++++++++++++- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/open_ended_grading/peer_grading_service.py b/lms/djangoapps/open_ended_grading/peer_grading_service.py index 8e0f8cbbaa..0ae4cc4534 100644 --- a/lms/djangoapps/open_ended_grading/peer_grading_service.py +++ b/lms/djangoapps/open_ended_grading/peer_grading_service.py @@ -29,6 +29,15 @@ This is a mock peer grading service that can be used for unit tests without making actual service calls to the grading controller """ class MockPeerGradingService(object): + # TODO: get this rubric parsed and working + rubric = """ + + Description + + + + """ + def get_next_submission(self, problem_location, grader_id): return json.dumps({'success': True, 'submission_id':1, @@ -56,15 +65,15 @@ class MockPeerGradingService(object): def save_calibration_essay(self, problem_location, grader_id, calibration_essay_id, submission_key, score, feedback): - return {'success': True, 'actual_score': 2} + return json.dumps({'success': True, 'actual_score': 2}) def get_problem_list(self, course_id, grader_id): return json.dumps({'success': True, 'problem_list': [ json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo1', - 'problem_name': "Problem 1", 'num_graded': 3, 'num_pending': 5}), + 'problem_name': "Problem 1", 'num_graded': 3, 'num_pending': 5, 'num_required': 7}), json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo2', - 'problem_name': "Problem 2", 'num_graded': 1, 'num_pending': 5}) + 'problem_name': "Problem 2", 'num_graded': 1, 'num_pending': 5, 'num_required': 8}) ]}) class PeerGradingService(GradingService): diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index bd2b3c6695..d1670a1b26 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -18,6 +18,8 @@ from nose import SkipTest from mock import patch, Mock import json +import logging +log = logging.getLogger(__name__) from override_settings import override_settings @@ -182,7 +184,7 @@ class TestPeerGradingService(ct.PageLoader): self.assertFalse(d['success']) self.assertTrue(d['error'].find('Missing required keys:') > -1) - def test_is_calibrated(self): + def test_is_calibrated_success(self): self.login(self.student, self.password) url = reverse('peer_grading_is_student_calibrated', kwargs={'course_id': self.course_id}) data = {'location': self.location} @@ -191,3 +193,61 @@ class TestPeerGradingService(ct.PageLoader): self.assertTrue(d['success']) self.assertTrue('calibrated' in d) + def test_is_calibrated_failure(self): + self.login(self.student, self.password) + url = reverse('peer_grading_is_student_calibrated', kwargs={'course_id': self.course_id}) + data = {} + r = self.check_for_post_code(200, url, data) + d = json.loads(r.content) + self.assertFalse(d['success']) + self.assertFalse('calibrated' in d) + + def test_show_calibration_essay_success(self): + self.login(self.student, self.password) + + url = reverse('peer_grading_show_calibration_essay', kwargs={'course_id': self.course_id}) + data = {'location': self.location} + + r = self.check_for_post_code(200, url, data) + d = json.loads(r.content) + self.assertTrue(d['success']) + self.assertIsNotNone(d['submission_id']) + self.assertIsNotNone(d['prompt']) + self.assertIsNotNone(d['submission_key']) + self.assertIsNotNone(d['max_score']) + + def test_show_calibration_essay_missing_key(self): + self.login(self.student, self.password) + + url = reverse('peer_grading_show_calibration_essay', kwargs={'course_id': self.course_id}) + data = {} + + r = self.check_for_post_code(200, url, data) + d = json.loads(r.content) + + self.assertFalse(d['success']) + self.assertEqual(d['error'], "Missing required keys: location") + + def test_save_calibration_essay_success(self): + self.login(self.student, self.password) + url = reverse('peer_grading_save_calibration_essay', kwargs={'course_id': self.course_id}) + data = {'location': self.location, + 'submission_id': '1', + 'submission_key': 'fake key', + 'score': '2', + 'feedback': 'This is feedback'} + r = self.check_for_post_code(200, url, data) + d = json.loads(r.content) + self.assertTrue(d['success']) + self.assertTrue('actual_score' in d) + + def test_save_calibration_essay_missing_keys(self): + self.login(self.student, self.password) + url = reverse('peer_grading_save_calibration_essay', kwargs={'course_id': self.course_id}) + data = {} + r = self.check_for_post_code(200, url, data) + d = json.loads(r.content) + self.assertFalse(d['success']) + self.assertTrue(d['error'].find('Missing required keys:') > -1) + self.assertFalse('actual_score' in d) + From 1f8db18903efe30a21816de5543abc134d6ecd93 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 24 Jan 2013 12:36:48 -0500 Subject: [PATCH 04/60] New tests for open ended module --- .../xmodule/tests/test_combined_open_ended.py | 73 +++++++++++++++++++ .../xmodule/tests/test_self_assessment.py | 59 ++++++++------- 2 files changed, 106 insertions(+), 26 deletions(-) create mode 100644 common/lib/xmodule/xmodule/tests/test_combined_open_ended.py diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py new file mode 100644 index 0000000000..12ad862a43 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -0,0 +1,73 @@ +import json +from mock import Mock +import unittest + +from xmodule.openendedchild import OpenEndedChild +from xmodule.modulestore import Location +from lxml import etree + +from . import test_system + +class OpenEndedChildTest(unittest.TestCase): + location = Location(["i4x", "edX", "sa_test", "selfassessment", + "SampleQuestion"]) + + metadata = json.dumps({'attempts': '10'}) + prompt = etree.XML("This is a question prompt") + rubric = ''' + + Response Quality + + + ''' + max_score = 4 + + static_data = { + 'max_attempts': 20, + 'prompt': prompt, + 'rubric': rubric, + 'max_score': max_score, + } + definition = Mock() + descriptor = Mock() + + + def test_latest_answer_empty(self): + openendedchild = OpenEndedChild(test_system, self.location, + self.definition, self.descriptor, self.static_data, self.metadata) + answer = openendedchild.latest_answer() + self.assertEqual(answer, "") + + def test_latest_answer_nonempty(self): + metadata = json.dumps({ 'attempts': 10, + 'history': [{'answer': "Two"}, {'answer': "Three"}]}) + openendedchild = OpenEndedChild(test_system, self.location, + self.definition, self.descriptor, self.static_data, metadata) + answer = openendedchild.latest_answer() + self.assertEqual(answer, "Three") + + def test_latest_score_empty(self): + openendedchild = OpenEndedChild(test_system, self.location, + self.definition, self.descriptor, self.static_data, self.metadata) + answer = openendedchild.latest_score() + self.assertEqual(answer, None) + + + def test_latest_score_nonempty(self): + metadata = json.dumps({ 'attempts': 10, + 'history': [{'score': 3}, {'score': 2}]}) + openendedchild = OpenEndedChild(test_system, self.location, + self.definition, self.descriptor, self.static_data, metadata) + answer = openendedchild.latest_score() + self.assertEqual(answer, 2) + + + def test_new_history_entry(self): + openendedchild = OpenEndedChild(test_system, self.location, + self.definition, self.descriptor, self.static_data, self.metadata) + new_answer = "New Answer" + openendedchild.new_history_entry(new_answer) + answer = openendedchild.latest_answer() + self.assertEqual(answer, new_answer) + + #def test_record_latest_score(self): diff --git a/common/lib/xmodule/xmodule/tests/test_self_assessment.py b/common/lib/xmodule/xmodule/tests/test_self_assessment.py index 565483c586..a11facb05b 100644 --- a/common/lib/xmodule/xmodule/tests/test_self_assessment.py +++ b/common/lib/xmodule/xmodule/tests/test_self_assessment.py @@ -10,8 +10,16 @@ from . import test_system class SelfAssessmentTest(unittest.TestCase): - definition = {'rubric': 'A rubric', - 'prompt': 'Who?', + rubric = ''' + + Response Quality + + + ''' + + prompt = etree.XML("This is sample prompt text.") + definition = {'rubric': rubric, + 'prompt': prompt, 'submitmessage': 'Shall we submit now?', 'hintprompt': 'Consider this...', } @@ -23,48 +31,47 @@ class SelfAssessmentTest(unittest.TestCase): descriptor = Mock() - def test_import(self): + def setUp(self): state = json.dumps({'student_answers': ["Answer 1", "answer 2", "answer 3"], 'scores': [0, 1], 'hints': ['o hai'], 'state': SelfAssessmentModule.INITIAL, 'attempts': 2}) - rubric = ''' - - Response Quality - - - ''' - - prompt = etree.XML("Text") static_data = { 'max_attempts': 10, - 'rubric': etree.XML(rubric), - 'prompt': prompt, + 'rubric': etree.XML(self.rubric), + 'prompt': self.prompt, 'max_score': 1 } - module = SelfAssessmentModule(test_system, self.location, + self.module = SelfAssessmentModule(test_system, self.location, self.definition, self.descriptor, static_data, state, metadata=self.metadata) - self.assertEqual(module.get_score()['score'], 0) + def test_get_html(self): + html = self.module.get_html(test_system) + self.assertTrue(html.find("This is sample prompt text") != -1) + + def test_self_assessment_flow(self): + + self.assertEqual(self.module.get_score()['score'], 0) - module.save_answer({'student_answer': "I am an answer"}, test_system) - self.assertEqual(module.state, module.ASSESSING) + self.module.save_answer({'student_answer': "I am an answer"}, test_system) + self.assertEqual(self.module.state, self.module.ASSESSING) - module.save_assessment({'assessment': '0'}, test_system) - self.assertEqual(module.state, module.POST_ASSESSMENT) - module.save_hint({'hint': 'this is a hint'}, test_system) - self.assertEqual(module.state, module.DONE) + self.module.save_assessment({'assessment': '0'}, test_system) + self.assertEqual(self.module.state, self.module.POST_ASSESSMENT) + self.module.save_hint({'hint': 'this is a hint'}, test_system) + self.assertEqual(self.module.state, self.module.DONE) - d = module.reset({}) + d = self.module.reset({}) self.assertTrue(d['success']) - self.assertEqual(module.state, module.INITIAL) + self.assertEqual(self.module.state, self.module.INITIAL) # if we now assess as right, skip the REQUEST_HINT state - module.save_answer({'student_answer': 'answer 4'}, test_system) - module.save_assessment({'assessment': '1'}, test_system) - self.assertEqual(module.state, module.DONE) + self.module.save_answer({'student_answer': 'answer 4'}, test_system) + self.module.save_assessment({'assessment': '1'}, test_system) + self.assertEqual(self.module.state, self.module.DONE) + From 37c7b470173c5bebab64c1c79a362f7e25f41092 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 24 Jan 2013 14:02:24 -0500 Subject: [PATCH 05/60] Update comments and add new OpenEndedChild tests --- common/lib/xmodule/xmodule/openendedchild.py | 4 +- .../xmodule/tests/test_combined_open_ended.py | 101 +++++++++++++----- 2 files changed, 78 insertions(+), 27 deletions(-) diff --git a/common/lib/xmodule/xmodule/openendedchild.py b/common/lib/xmodule/xmodule/openendedchild.py index 88fed61c6d..8d3eb3e617 100644 --- a/common/lib/xmodule/xmodule/openendedchild.py +++ b/common/lib/xmodule/xmodule/openendedchild.py @@ -112,7 +112,7 @@ class OpenEndedChild(object): pass def latest_answer(self): - """None if not available""" + """Empty string if not available""" if not self.history: return "" return self.history[-1].get('answer', "") @@ -124,7 +124,7 @@ class OpenEndedChild(object): return self.history[-1].get('score') def latest_post_assessment(self, system): - """None if not available""" + """Empty string if not available""" if not self.history: return "" return self.history[-1].get('post_assessment', "") 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 12ad862a43..c53317fe1f 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -31,43 +31,94 @@ class OpenEndedChildTest(unittest.TestCase): definition = Mock() descriptor = Mock() + def setUp(self): + self.openendedchild = OpenEndedChild(test_system, self.location, + self.definition, self.descriptor, self.static_data, self.metadata) + def test_latest_answer_empty(self): - openendedchild = OpenEndedChild(test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) - answer = openendedchild.latest_answer() + answer = self.openendedchild.latest_answer() self.assertEqual(answer, "") - def test_latest_answer_nonempty(self): - metadata = json.dumps({ 'attempts': 10, - 'history': [{'answer': "Two"}, {'answer': "Three"}]}) - openendedchild = OpenEndedChild(test_system, self.location, - self.definition, self.descriptor, self.static_data, metadata) - answer = openendedchild.latest_answer() - self.assertEqual(answer, "Three") def test_latest_score_empty(self): - openendedchild = OpenEndedChild(test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) - answer = openendedchild.latest_score() + answer = self.openendedchild.latest_score() self.assertEqual(answer, None) - def test_latest_score_nonempty(self): - metadata = json.dumps({ 'attempts': 10, - 'history': [{'score': 3}, {'score': 2}]}) - openendedchild = OpenEndedChild(test_system, self.location, - self.definition, self.descriptor, self.static_data, metadata) - answer = openendedchild.latest_score() - self.assertEqual(answer, 2) + def test_latest_post_assessment_empty(self): + answer = self.openendedchild.latest_post_assessment(test_system) + self.assertEqual(answer, "") def test_new_history_entry(self): - openendedchild = OpenEndedChild(test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) new_answer = "New Answer" - openendedchild.new_history_entry(new_answer) - answer = openendedchild.latest_answer() + self.openendedchild.new_history_entry(new_answer) + answer = self.openendedchild.latest_answer() self.assertEqual(answer, new_answer) - #def test_record_latest_score(self): + new_answer = "Newer Answer" + self.openendedchild.new_history_entry(new_answer) + answer = self.openendedchild.latest_answer() + self.assertEqual(new_answer, answer) + + def test_record_latest_score(self): + new_answer = "New Answer" + self.openendedchild.new_history_entry(new_answer) + new_score = 3 + self.openendedchild.record_latest_score(new_score) + score = self.openendedchild.latest_score() + self.assertEqual(score, 3) + + new_score = 4 + self.openendedchild.new_history_entry(new_answer) + self.openendedchild.record_latest_score(new_score) + 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) + + post_assessment = "Post assessment" + self.openendedchild.record_latest_post_assessment(post_assessment) + self.assertEqual(post_assessment, + self.openendedchild.latest_post_assessment(test_system)) + + def test_get_score(self): + new_answer = "New Answer" + self.openendedchild.new_history_entry(new_answer) + + score = self.openendedchild.get_score() + self.assertEqual(score['score'], 0) + self.assertEqual(score['total'], self.static_data['max_score']) + + new_score = 4 + self.openendedchild.new_history_entry(new_answer) + self.openendedchild.record_latest_score(new_score) + score = self.openendedchild.get_score() + self.assertEqual(score['score'], new_score) + self.assertEqual(score['total'], self.static_data['max_score']) + + + def test_reset(self): + self.openendedchild.reset(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') + + self.openendedchild.new_history_entry(new_answer) + self.openendedchild.record_latest_score(0) + self.assertEqual(self.openendedchild.is_last_response_correct(), + 'incorrect') + + + From bb60dde45e5dd03d8d90c47c03d06e518a6fc2ce Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 24 Jan 2013 17:09:19 -0500 Subject: [PATCH 06/60] Add OpenEndedModule tests --- .../xmodule/tests/test_combined_open_ended.py | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) 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 c53317fe1f..7ae17f6a4f 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -1,8 +1,9 @@ import json -from mock import Mock +from mock import Mock, MagicMock import unittest from xmodule.openendedchild import OpenEndedChild +from xmodule.open_ended_module import OpenEndedModule from xmodule.modulestore import Location from lxml import etree @@ -120,5 +121,59 @@ class OpenEndedChildTest(unittest.TestCase): self.assertEqual(self.openendedchild.is_last_response_correct(), 'incorrect') +class OpenEndedModuleTest(unittest.TestCase): + location = Location(["i4x", "edX", "sa_test", "selfassessment", + "SampleQuestion"]) + + metadata = json.dumps({'attempts': '10'}) + prompt = etree.XML("This is a question prompt") + rubric = etree.XML(''' + + Response Quality + + + ''') + max_score = 4 + + static_data = { + 'max_attempts': 20, + 'prompt': prompt, + 'rubric': rubric, + 'max_score': max_score, + } + + oeparam = etree.XML(''' + + Enter essay here. + This is the answer. + {"grader_settings" : "ml_grading.conf", "problem_id" : "6.002x/Welcome/OETest"} + + ''') + definition = {'oeparam': oeparam} + descriptor = Mock() + + def setUp(self): + test_system.location = self.location + self.mock_xqueue = MagicMock() + self.mock_xqueue.send_to_queue.return_value=(None, "Message") + test_system.xqueue = {'interface':self.mock_xqueue, 'callback_url':'/', 'default_queuename': 'testqueue', 'waittime': 1} + self.openendedmodule = OpenEndedModule(test_system, self.location, + self.definition, self.descriptor, self.static_data, self.metadata) + + def test_message_post(self): + get = {'feedback': 'feedback text', + 'submission_id': '1', + 'grader_id': '1', + 'score': 3} + + result = self.openendedmodule.message_post(get, test_system) + self.assertTrue(result['success']) + state = json.loads(self.openendedmodule.get_instance_state()) + self.assertIsNotNone(state['state'], OpenEndedModule.DONE) + + def test_send_to_grader(self): + result = self.openendedmodule.send_to_grader("This is a student submission", test_system) + self.assertTrue(result) + From bb521a20b7f33743bd52a512573a29d0580aaf74 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 25 Jan 2013 10:19:09 -0500 Subject: [PATCH 07/60] fix a bug caught with testing and update some tests (the last couple are still broken, though) --- .../lib/xmodule/xmodule/open_ended_module.py | 6 +- .../xmodule/tests/test_combined_open_ended.py | 69 ++++++++++++++++++- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_module.py index 0eaad34bad..0b7523f984 100644 --- a/common/lib/xmodule/xmodule/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_module.py @@ -257,7 +257,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): """ new_score_msg = self._parse_score_msg(score_msg, system) if not new_score_msg['valid']: - score_msg['feedback'] = 'Invalid grader reply. Please contact the course staff.' + new_score_msg['feedback'] = 'Invalid grader reply. Please contact the course staff.' self.record_latest_score(new_score_msg['score']) self.record_latest_post_assessment(score_msg) @@ -404,6 +404,10 @@ class OpenEndedModule(openendedchild.OpenEndedChild): 'score': Numeric value (floating point is okay) to assign to answer 'msg': grader_msg 'feedback' : feedback from grader + 'grader_type': what type of grader resulted in this score + 'grader_id': id of the grader + 'submission_id' : id of the submission + 'success': whether or not this submission was successful } Returns (valid_score_msg, correct, score, msg): 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 7ae17f6a4f..18176d0e12 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -1,11 +1,13 @@ import json -from mock import Mock, MagicMock +from mock import Mock, MagicMock, ANY import unittest from xmodule.openendedchild import OpenEndedChild from xmodule.open_ended_module import OpenEndedModule from xmodule.modulestore import Location from lxml import etree +import capa.xqueue_interface as xqueue_interface +from datetime import datetime from . import test_system @@ -165,15 +167,78 @@ class OpenEndedModuleTest(unittest.TestCase): 'submission_id': '1', 'grader_id': '1', 'score': 3} + qtime = datetime.strftime(datetime.now(), xqueue_interface.dateformat) + student_info = {'anonymous_student_id': test_system.anonymous_student_id, + 'submission_time': qtime} + contents = { + 'feedback': get['feedback'], + 'submission_id': int(get['submission_id']), + 'grader_id': int(get['grader_id']), + 'score': get['score'], + 'student_info': json.dumps(student_info) + } result = self.openendedmodule.message_post(get, test_system) self.assertTrue(result['success']) + # make sure it's actually sending something to the queue + self.mock_xqueue.send_to_queue.assert_called_with(body = json.dumps(contents), header=ANY) + state = json.loads(self.openendedmodule.get_instance_state()) self.assertIsNotNone(state['state'], OpenEndedModule.DONE) def test_send_to_grader(self): - result = self.openendedmodule.send_to_grader("This is a student submission", test_system) + submission = "This is a student submission" + qtime = datetime.strftime(datetime.now(), xqueue_interface.dateformat) + student_info = {'anonymous_student_id': test_system.anonymous_student_id, + 'submission_time': qtime} + contents = self.openendedmodule.payload.copy() + contents.update({ + 'student_info': json.dumps(student_info), + 'student_response': submission, + 'max_score': self.max_score + }) + result = self.openendedmodule.send_to_grader(submission, test_system) self.assertTrue(result) + self.mock_xqueue.send_to_queue.assert_called_with(body = json.dumps(contents), header=ANY) + + def update_score_single(self): + self.openendedmodule.new_history_entry("New Entry") + score_msg = { + 'correct': True, + 'score': 4, + 'msg' : 'Grader Message', + 'feedback': "Grader Feedback" + } + get = {'queuekey': "abcd", + 'xqueue_body': score_msg} + self.openendedmodule.update_score(get, test_system) + + def update_score_single(self): + self.openendedmodule.new_history_entry("New Entry") + feedback = { + "success": True, + "feedback": "Grader Feedback" + } + score_msg = { + 'correct': True, + 'score': 4, + 'msg' : 'Grader Message', + 'feedback': feedback, + 'grader_type': 'IN', + 'grader_id': '1', + 'submission_id': '1', + 'success': True + } + get = {'queuekey': "abcd", + 'xqueue_body': json.dumps(score_msg)} + self.openendedmodule.update_score(get, test_system) + def test_update_score(self): + self.update_score_single() + score = self.openendedmodule.latest_score() + self.assertEqual(score, 4) + + def test_latest_post_assessment(self): + self.update_score_single() From 71d27def0d28f41eb2283f848dce22defb47e292 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 25 Jan 2013 11:18:30 -0500 Subject: [PATCH 08/60] Fix tests and add new documentation --- .../lib/xmodule/xmodule/open_ended_module.py | 3 +++ .../xmodule/tests/test_combined_open_ended.py | 27 ++++++++++++++----- .../xmodule/tests/test_self_assessment.py | 2 +- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_module.py index 7c3293730c..f32e0b26ff 100644 --- a/common/lib/xmodule/xmodule/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_module.py @@ -410,6 +410,9 @@ class OpenEndedModule(openendedchild.OpenEndedChild): 'grader_id': id of the grader 'submission_id' : id of the submission 'success': whether or not this submission was successful + 'rubric_scores': a list of rubric scores + 'rubric_scores_complete': boolean if rubric scores are complete + 'rubric_xml': the xml of the rubric in string format } Returns (valid_score_msg, correct, score, msg): 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 138424f021..847ac108dc 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -10,6 +10,13 @@ import capa.xqueue_interface as xqueue_interface from datetime import datetime from . import test_system +""" +Tests for the various pieces of the CombinedOpenEndedGrading system + +OpenEndedChild +OpenEndedModule + +""" class OpenEndedChildTest(unittest.TestCase): location = Location(["i4x", "edX", "sa_test", "selfassessment", @@ -130,12 +137,12 @@ class OpenEndedModuleTest(unittest.TestCase): metadata = json.dumps({'attempts': '10'}) prompt = etree.XML("This is a question prompt") - rubric = etree.XML(''' + rubric = etree.XML(''' Response Quality - ''') + ''') max_score = 4 static_data = { @@ -225,22 +232,28 @@ class OpenEndedModuleTest(unittest.TestCase): 'correct': True, 'score': 4, 'msg' : 'Grader Message', - 'feedback': feedback, + 'feedback': json.dumps(feedback), 'grader_type': 'IN', 'grader_id': '1', 'submission_id': '1', - 'success': True + 'success': True, + 'rubric_scores': [0], + 'rubric_scores_complete': True, + 'rubric_xml': etree.tostring(self.rubric) } get = {'queuekey': "abcd", 'xqueue_body': json.dumps(score_msg)} self.openendedmodule.update_score(get, test_system) - + def test_latest_post_assessment(self): + self.update_score_single() + assessment = self.openendedmodule.latest_post_assessment(test_system) + self.assertFalse(assessment == '') + # check for errors + self.assertFalse('errors' in assessment) def test_update_score(self): self.update_score_single() score = self.openendedmodule.latest_score() self.assertEqual(score, 4) - def test_latest_post_assessment(self): - self.update_score_single() diff --git a/common/lib/xmodule/xmodule/tests/test_self_assessment.py b/common/lib/xmodule/xmodule/tests/test_self_assessment.py index 55c198486c..9e138fef19 100644 --- a/common/lib/xmodule/xmodule/tests/test_self_assessment.py +++ b/common/lib/xmodule/xmodule/tests/test_self_assessment.py @@ -52,7 +52,7 @@ class SelfAssessmentTest(unittest.TestCase): def test_get_html(self): html = self.module.get_html(test_system) - self.assertTrue(html.find("This is sample prompt text") != -1) + self.assertTrue("This is sample prompt text" in html) def test_self_assessment_flow(self): From a26006f7c32adbf68d4e2f766073ff941586c5e2 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 25 Jan 2013 15:09:01 -0500 Subject: [PATCH 09/60] Add new test for the CombinedOpenEndedModule --- .../xmodule/tests/test_combined_open_ended.py | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) 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 847ac108dc..3fb3cbe6d2 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -4,6 +4,8 @@ import unittest from xmodule.openendedchild import OpenEndedChild from xmodule.open_ended_module import OpenEndedModule +from xmodule.combined_open_ended_module import CombinedOpenEndedModule + from xmodule.modulestore import Location from lxml import etree import capa.xqueue_interface as xqueue_interface @@ -189,7 +191,7 @@ class OpenEndedModuleTest(unittest.TestCase): result = self.openendedmodule.message_post(get, test_system) self.assertTrue(result['success']) - # make sure it's actually sending something to the queue + # make sure it's actually sending something we want to the queue self.mock_xqueue.send_to_queue.assert_called_with(body = json.dumps(contents), header=ANY) state = json.loads(self.openendedmodule.get_instance_state()) @@ -257,3 +259,53 @@ class OpenEndedModuleTest(unittest.TestCase): score = self.openendedmodule.latest_score() self.assertEqual(score, 4) +class CombinedOpenEndedModuleTest(unittest.TestCase): + location = Location(["i4x", "edX", "open_ended", "combinedopenended", + "SampleQuestion"]) + + metadata = json.dumps({'attempts': '10'}) + prompt = "This is a question prompt" + rubric = ''' + + Response Quality + + + ''' + max_score = 4 + + static_data = json.dumps({ + 'max_attempts': 20, + 'prompt': prompt, + 'rubric': rubric, + 'max_score': max_score, + 'display_name': 'Name' + }) + + oeparam = etree.XML(''' + + Enter essay here. + This is the answer. + {"grader_settings" : "ml_grading.conf", "problem_id" : "6.002x/Welcome/OETest"} + + ''') + + task_xml = ''' + + + What hint about this problem would you give to someone? + + + Save Succcesful. Thanks for participating! + + + ''' + definition = {'prompt': etree.XML(prompt), 'rubric': etree.XML(rubric), 'task_xml': [task_xml]} + descriptor = Mock() + + def setUp(self): + self.combinedoe = CombinedOpenEndedModule(test_system, self.location, self.definition, self.descriptor, self.static_data, self.metadata) + + def test_get_tag_name(self): + name = self.combinedoe.get_tag_name("Tag") + self.assertEqual(name, "t") + From 9f6afcb606a77f8adad07e33e4275eae33023739 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 25 Jan 2013 16:33:45 -0500 Subject: [PATCH 10/60] Add some more tests --- .../xmodule/tests/test_combined_open_ended.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) 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 3fb3cbe6d2..ba3dc10b4b 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -263,7 +263,6 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestion"]) - metadata = json.dumps({'attempts': '10'}) prompt = "This is a question prompt" rubric = ''' @@ -273,6 +272,8 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): ''' max_score = 4 + metadata = {'attempts': '10', 'max_score': max_score} + static_data = json.dumps({ 'max_attempts': 20, 'prompt': prompt, @@ -303,9 +304,23 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): descriptor = Mock() def setUp(self): - self.combinedoe = CombinedOpenEndedModule(test_system, self.location, self.definition, self.descriptor, self.static_data, self.metadata) + self.combinedoe = CombinedOpenEndedModule(test_system, self.location, self.definition, self.descriptor, self.static_data, metadata=self.metadata) def test_get_tag_name(self): name = self.combinedoe.get_tag_name("Tag") self.assertEqual(name, "t") + def test_get_last_response(self): + response_dict = self.combinedoe.get_last_response(0) + self.assertEqual(response_dict['type'], "selfassessment") + self.assertEqual(response_dict['max_score'], self.max_score) + self.assertEqual(response_dict['state'], CombinedOpenEndedModule.INITIAL) + + def test_update_task_states(self): + changed = self.combinedoe.update_task_states() + self.assertFalse(changed) + + # do something to change the state + + # check again + From 8d64bfdbdb910535fa41e130b7a2af9a7eb7aa66 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 28 Jan 2013 08:41:39 -0500 Subject: [PATCH 11/60] Update tests to handle rubric scores --- lms/djangoapps/open_ended_grading/peer_grading_service.py | 4 ++-- lms/djangoapps/open_ended_grading/tests.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/open_ended_grading/peer_grading_service.py b/lms/djangoapps/open_ended_grading/peer_grading_service.py index b850293255..b4f918397f 100644 --- a/lms/djangoapps/open_ended_grading/peer_grading_service.py +++ b/lms/djangoapps/open_ended_grading/peer_grading_service.py @@ -50,7 +50,7 @@ class MockPeerGradingService(object): 'max_score': 4}) def save_grade(self, location, grader_id, submission_id, - score, feedback, submission_key): + score, feedback, submission_key, rubric_scores): return json.dumps({'success': True}) def is_student_calibrated(self, problem_location, grader_id): @@ -66,7 +66,7 @@ class MockPeerGradingService(object): 'max_score': 4}) def save_calibration_essay(self, problem_location, grader_id, - calibration_essay_id, submission_key, score, feedback): + calibration_essay_id, submission_key, score, feedback, rubric_scores): return json.dumps({'success': True, 'actual_score': 2}) def get_problem_list(self, course_id, grader_id): diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index c1d192b6cc..57ea4f319c 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -171,7 +171,8 @@ class TestPeerGradingService(ct.PageLoader): 'submission_id': '1', 'submission_key': 'fake key', 'score': '2', - 'feedback': 'This is feedback'} + 'feedback': 'This is feedback', + 'rubric_scores[]': [1, 2]} r = self.check_for_post_code(200, url, data) d = json.loads(r.content) self.assertTrue(d['success']) @@ -236,7 +237,8 @@ class TestPeerGradingService(ct.PageLoader): 'submission_id': '1', 'submission_key': 'fake key', 'score': '2', - 'feedback': 'This is feedback'} + 'feedback': 'This is feedback', + 'rubric_scores[]': [1, 2]} r = self.check_for_post_code(200, url, data) d = json.loads(r.content) self.assertTrue(d['success']) From fe4dc7622ea1afb325d49cb04358e1357fc81957 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 28 Jan 2013 10:18:01 -0500 Subject: [PATCH 12/60] Updates to combined open ended tests --- .../xmodule/tests/test_combined_open_ended.py | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) 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 ba3dc10b4b..b327d10340 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -290,7 +290,7 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): ''') - task_xml = ''' + task_xml1 = ''' What hint about this problem would you give to someone? @@ -300,7 +300,15 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): ''' - definition = {'prompt': etree.XML(prompt), 'rubric': etree.XML(rubric), 'task_xml': [task_xml]} + task_xml2 = ''' + + + Enter essay here. + This is the answer. + {"grader_settings" : "ml_grading.conf", "problem_id" : "6.002x/Welcome/OETest"} + + ''' + definition = {'prompt': etree.XML(prompt), 'rubric': etree.XML(rubric), 'task_xml': [task_xml1, task_xml2]} descriptor = Mock() def setUp(self): @@ -320,7 +328,12 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): changed = self.combinedoe.update_task_states() self.assertFalse(changed) - # do something to change the state + #prev_context = self.combinedoe.get_context() + #new_state = {'state': CombinedOpenEndedModule.INITIAL, 'history': []} + ## force a task change + #task_state = self.combinedoe.overwrite_state(json.dumps(new_state)) - # check again + ## check again + #new_context = self.combinedoe.get_context() + #self.assertNotEqual(prev_context['state'], new_context['state']) From ffbda8a0be72c8add8b6210a6437d9f178148bdf Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 28 Jan 2013 11:52:15 -0500 Subject: [PATCH 13/60] Update test --- .../xmodule/tests/test_combined_open_ended.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) 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 b327d10340..9485ed2a47 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -328,12 +328,10 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): changed = self.combinedoe.update_task_states() self.assertFalse(changed) - #prev_context = self.combinedoe.get_context() - #new_state = {'state': CombinedOpenEndedModule.INITIAL, 'history': []} - ## force a task change - #task_state = self.combinedoe.overwrite_state(json.dumps(new_state)) + current_task = self.combinedoe.current_task + current_task.change_state(CombinedOpenEndedModule.DONE) + changed = self.combinedoe.update_task_states() + + self.assertTrue(changed) - ## check again - #new_context = self.combinedoe.get_context() - #self.assertNotEqual(prev_context['state'], new_context['state']) From 28a2dd9a18e1aafe14d26995ad0cfcacb83f6d6e Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 28 Jan 2013 16:33:28 -0500 Subject: [PATCH 14/60] support reordering of static tabs in studio --- cms/djangoapps/contentstore/views.py | 41 +++++++++++++++++++++++-- cms/static/coffee/src/views/tabs.coffee | 16 +++++++++- cms/urls.py | 1 + 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 816ccab091..326b47ee64 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -903,6 +903,36 @@ def static_pages(request, org, course, coursename): def edit_static(request, org, course, coursename): return render_to_response('edit-static-page.html', {}) + +@login_required +@expect_json +def reorder_tabs(request): + tabs = request.POST['tabs'] + logging.debug('tabs = {0} {1}'.format(tabs.__class__, tabs)) + + if len(tabs) > 0: + course = get_course_for_item(tabs[0]) + + if not has_access(request.user, course.location): + raise PermissionDenied() + + # first filter out all non-static tabs from the tabs list + course_tabs = [t for t in course.tabs if t['type'] != 'static_tab'] + + # OK, re-assemble the static tabs in the new order + for tab in tabs: + item = modulestore('direct').get_item(Location(tab)) + + course_tabs.append({ 'type':'static_tab', + 'name' : item.metadata.get('display_name'), + 'url_slug' : item.location.name} + ) + + course.tabs = course_tabs + modulestore('direct').update_metadata(course.location, course.metadata) + + return HttpResponse() + @login_required @ensure_csrf_cookie def edit_tabs(request, org, course, coursename): @@ -914,12 +944,19 @@ def edit_tabs(request, org, course, coursename): if not has_access(request.user, location): raise PermissionDenied() - static_tabs = modulestore('direct').get_items(static_tabs_loc) - # see tabs have been uninitialized (e.g. supporing courses created before tab support in studio) if course_item.tabs is None or len(course_item.tabs) == 0: initialize_course_tabs(course_item) + # first get all static tabs from the tabs list + # we do this because this is also the order in which items are displayed in the LMS + static_tabs_refs = [t for t in course_item.tabs if t['type'] == 'static_tab'] + + static_tabs = [] + for static_tab_ref in static_tabs_refs: + static_tab_loc = Location(location)._replace(category='static_tab', name=static_tab_ref['url_slug']) + static_tabs.append(modulestore('direct').get_item(static_tab_loc)) + components = [ static_tab.location.url() for static_tab diff --git a/cms/static/coffee/src/views/tabs.coffee b/cms/static/coffee/src/views/tabs.coffee index 1fbc6ffa7f..3799eb25ee 100644 --- a/cms/static/coffee/src/views/tabs.coffee +++ b/cms/static/coffee/src/views/tabs.coffee @@ -15,7 +15,7 @@ class CMS.Views.TabsEdit extends Backbone.View @$('.components').sortable( handle: '.drag-handle' - update: (event, ui) => alert 'not yet implemented!' + update: @tabMoved helper: 'clone' opacity: '0.5' placeholder: 'component-placeholder' @@ -24,6 +24,20 @@ class CMS.Views.TabsEdit extends Backbone.View items: '> .component' ) + tabMoved: (event, ui) => + tabs = [] + @$('.component').each((idx, element) => + tabs.push($(element).data('id')) + ) + $.ajax({ + type:'POST', + url: '/reorder_tabs', + data: JSON.stringify({ + tabs : tabs + }), + contentType: 'application/json' + }) + addNewTab: (event) => event.preventDefault() diff --git a/cms/urls.py b/cms/urls.py index 6f8736551b..2b329dc16b 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -17,6 +17,7 @@ urlpatterns = ('', url(r'^publish_draft$', 'contentstore.views.publish_draft', name='publish_draft'), url(r'^unpublish_unit$', 'contentstore.views.unpublish_unit', name='unpublish_unit'), url(r'^create_new_course', 'contentstore.views.create_new_course', name='create_new_course'), + url(r'^reorder_tabs', 'contentstore.views.reorder_tabs', name='reorder_tabs'), url(r'^(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', 'contentstore.views.course_index', name='course_index'), From 3f4c45468fcb68301aad66fbc74ef5970b99f5cf Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 29 Jan 2013 11:36:10 -0500 Subject: [PATCH 15/60] Fix courseware jasmine tests --- lms/static/coffee/spec/courseware_spec.coffee | 22 +++---------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/lms/static/coffee/spec/courseware_spec.coffee b/lms/static/coffee/spec/courseware_spec.coffee index 3b40930b41..a51e64ec21 100644 --- a/lms/static/coffee/spec/courseware_spec.coffee +++ b/lms/static/coffee/spec/courseware_spec.coffee @@ -10,19 +10,6 @@ describe 'Courseware', -> Courseware.start() expect(Logger.bind).toHaveBeenCalled() - describe 'bind', -> - beforeEach -> - @courseware = new Courseware - setFixtures """ -
-
-
- """ - - it 'binds the content change event', -> - @courseware.bind() - expect($('.course-content .sequence')).toHandleWith 'contentChanged', @courseware.render - describe 'render', -> beforeEach -> jasmine.stubRequests() @@ -30,6 +17,7 @@ describe 'Courseware', -> spyOn(window, 'Histogram') spyOn(window, 'Problem') spyOn(window, 'Video') + spyOn(XModule, 'loadModules') setFixtures """
@@ -41,12 +29,8 @@ describe 'Courseware', -> """ @courseware.render() - it 'detect the video elements and convert them', -> - expect(window.Video).toHaveBeenCalledWith('1', '1.0:abc1234') - expect(window.Video).toHaveBeenCalledWith('2', '1.0:def5678') - - it 'detect the problem element and convert it', -> - expect(window.Problem).toHaveBeenCalledWith(3, 'problem_3', '/example/url/') + it 'ensure that the XModules have been loaded', -> + expect(XModule.loadModules).toHaveBeenCalled() it 'detect the histrogram element and convert it', -> expect(window.Histogram).toHaveBeenCalledWith('3', [[0, 1]]) From 855c8bb7e7c7f9a6f1258d3e13e0f7a1cc6d8037 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 29 Jan 2013 13:44:42 -0500 Subject: [PATCH 16/60] add unit test for tab reordering --- cms/djangoapps/contentstore/tests/tests.py | 28 +++++++++++++++++++ cms/djangoapps/contentstore/views.py | 1 - .../data/full/policies/6.002_Spring_2012.json | 1 + 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 3025ee78a4..bfdcfe1438 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -350,6 +350,34 @@ class ContentStoreTest(TestCase): def test_edit_unit_full(self): self.check_edit_unit('full') + def test_static_tab_reordering(self): + import_from_xml(modulestore(), 'common/test/data/', ['full']) + + # reverse the ordering + tabs = { 'tabs' : ['i4x://edX/full/static_tab/resources', 'i4x://edX/full/static_tab/syllabus'] } + resp = self.client.post(reverse('reorder_tabs'), json.dumps(tabs), "application/json") + + ms = modulestore('direct') + course = ms.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 + + resource_idx = 0 + syllabus_idx = 0 + idx = 0 + for tab in course.tabs: + if tab['type'] == 'static_tab': + if tab['url_slug'] == 'resources': + resource_idx = idx + elif tab['url_slug'] == 'syllabus': + syllabus_idx = idx + idx+=1 + + self.assertLess(resource_idx, syllabus_idx) + + + + + 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 diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 326b47ee64..205bf787fd 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -908,7 +908,6 @@ def edit_static(request, org, course, coursename): @expect_json def reorder_tabs(request): tabs = request.POST['tabs'] - logging.debug('tabs = {0} {1}'.format(tabs.__class__, tabs)) if len(tabs) > 0: course = get_course_for_item(tabs[0]) diff --git a/common/test/data/full/policies/6.002_Spring_2012.json b/common/test/data/full/policies/6.002_Spring_2012.json index 345309ff5c..2f55528b7b 100644 --- a/common/test/data/full/policies/6.002_Spring_2012.json +++ b/common/test/data/full/policies/6.002_Spring_2012.json @@ -8,6 +8,7 @@ {"type": "courseware"}, {"type": "course_info", "name": "Course Info"}, {"type": "static_tab", "url_slug": "syllabus", "name": "Syllabus"}, + {"type": "static_tab", "url_slug": "resources", "name": "Resources"}, {"type": "discussion", "name": "Discussion"}, {"type": "wiki", "name": "Wiki"}, {"type": "progress", "name": "Progress"} From efc572c24d7fc6d4f99a0b3fd39deb05b064e80b Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 29 Jan 2013 14:36:27 -0500 Subject: [PATCH 17/60] Fix navigation jasmine tests --- lms/static/coffee/spec/navigation_spec.coffee | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lms/static/coffee/spec/navigation_spec.coffee b/lms/static/coffee/spec/navigation_spec.coffee index cb98c2b64c..1340984e52 100644 --- a/lms/static/coffee/spec/navigation_spec.coffee +++ b/lms/static/coffee/spec/navigation_spec.coffee @@ -16,6 +16,7 @@ describe 'Navigation', -> active: 1 header: 'h3' autoHeight: false + heightStyle: 'content' describe 'when there is no active section', -> beforeEach -> @@ -23,11 +24,12 @@ describe 'Navigation', -> $('#accordion').append('
') new Navigation - it 'activate the accordian with section 1 as active', -> + it 'activate the accordian with no section as active', -> expect($('#accordion').accordion).toHaveBeenCalledWith - active: 1 + active: 0 header: 'h3' autoHeight: false + heightStyle: 'content' it 'binds the accordionchange event', -> Navigation.bind() From 585f1c41d61aa962220c0d604058077093a77e95 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 29 Jan 2013 16:04:43 -0500 Subject: [PATCH 18/60] New jasmine tests for combined open ended problems. --- .../js/fixtures/combined-open-ended.html | 56 +++++++++++++++++++ .../combinedopenended/display_spec.coffee | 15 +++++ 2 files changed, 71 insertions(+) create mode 100644 common/lib/xmodule/xmodule/js/fixtures/combined-open-ended.html create mode 100644 common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee diff --git a/common/lib/xmodule/xmodule/js/fixtures/combined-open-ended.html b/common/lib/xmodule/xmodule/js/fixtures/combined-open-ended.html new file mode 100644 index 0000000000..c7170d3472 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/fixtures/combined-open-ended.html @@ -0,0 +1,56 @@ +
+ +

Problem 1

+
+

Status

+
+
+ +
+ + Step 1 (Problem complete) : 1 / 1 + + +
+ +
+ + Step 2 (Being scored) : None / 1 + + +
+
+
+ +
+ +
+

Problem

+
+
+
+ + Some prompt. + +
+
+
+ Submitted for grading. + +
+ +
+ + +
+
+ + + + +
+ + +
+
+
diff --git a/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee b/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee new file mode 100644 index 0000000000..60c73cf1fe --- /dev/null +++ b/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee @@ -0,0 +1,15 @@ +xdescribe 'CombinedOpenEnded', -> + beforeEach -> + spyOn Logger, 'log' + # load up some fixtures + loadFixtures 'combined-open-ended.html' + @element = $('.combined-open-ended') + + describe 'constructor', -> + beforeEach -> + @combined = new CombinedOpenEnded @element + + it 'set the element', -> + except(@combined.element).not.toEqual @element + + #it 'initialize the ajax url, state, and task count' From 26cb1c5a2564ec85d3dea8f762c559397fe001b5 Mon Sep 17 00:00:00 2001 From: cahrens Date: Tue, 29 Jan 2013 16:30:41 -0500 Subject: [PATCH 19/60] Fixes for https://edx.lighthouseapp.com/projects/102637/tickets/144 (progress tab not updating). --- common/lib/xmodule/xmodule/capa_module.py | 1 + lms/djangoapps/courseware/views.py | 7 +++++-- lms/templates/problem_ajax.html | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index e4ab804f49..f33da6e3a4 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -229,6 +229,7 @@ class CapaModule(XModule): 'element_id': self.location.html_id(), 'id': self.id, 'ajax_url': self.system.ajax_url, + 'progress': Progress.to_js_status_str(self.get_progress()) }) def get_problem_html(self, encapsulate=True): diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 760ccb1d05..8c529a8585 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -233,10 +233,13 @@ def index(request, course_id, chapter=None, section=None, # Specifically asked-for section doesn't exist raise Http404 - # Load all descendents of the section, because we're going to display it's + # Load all descendants of the section, because we're going to display it's # html, which in general will need all of its children + section_module_cache = StudentModuleCache.cache_for_descriptor_descendents( + course.id, request.user, section_descriptor, depth=None) + section_module = get_module(request.user, request, section_descriptor.location, - student_module_cache, course.id, position=position, depth=None) + section_module_cache, course.id, position=position, depth=None) if section_module is None: # User may be trying to be clever and access something # they don't have access to. diff --git a/lms/templates/problem_ajax.html b/lms/templates/problem_ajax.html index 012e4276c3..42cd18c4e3 100644 --- a/lms/templates/problem_ajax.html +++ b/lms/templates/problem_ajax.html @@ -1 +1 @@ -
+
From b8c1dba1cba892a863bb84b4249b193a1711cbe1 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Tue, 29 Jan 2013 17:31:12 -0500 Subject: [PATCH 20/60] Catch save error and throw it in the user's face (bug 147). May need to add catches in more places or put in $.ajaxSetup? --- cms/static/js/views/settings/main_settings_view.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cms/static/js/views/settings/main_settings_view.js b/cms/static/js/views/settings/main_settings_view.js index 9037d4510c..704623b56e 100644 --- a/cms/static/js/views/settings/main_settings_view.js +++ b/cms/static/js/views/settings/main_settings_view.js @@ -55,7 +55,10 @@ CMS.Views.ValidatingView = Backbone.View.extend({ var newVal = $(event.currentTarget).val(); if (currentVal != newVal) { this.clearValidationErrors(); - this.model.save(field, newVal); + this.model.save(field, newVal, { error : function(model, error) { + // this handler is for the client:server communication not the vlidation errors which handleValidationError catches + if (error.responseText) window.alert("Error: " + error.responseText); + }}); return true; } else return false; From f1973dc660a5de9bfa24300a51e032fde151ee9c Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 29 Jan 2013 17:34:54 -0500 Subject: [PATCH 21/60] minor update to new jasmine test --- .../xmodule/js/spec/combinedopenended/display_spec.coffee | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee b/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee index 60c73cf1fe..3ccf504948 100644 --- a/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee @@ -12,4 +12,5 @@ xdescribe 'CombinedOpenEnded', -> it 'set the element', -> except(@combined.element).not.toEqual @element - #it 'initialize the ajax url, state, and task count' + #it 'initialize the ajax url, state, and task count', -> + From d132f2e18a628caf972e58b4e6be1bd1ab93e3ee Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 28 Jan 2013 15:05:04 -0500 Subject: [PATCH 22/60] Only put data repos in STATICFILES_DIRS if we're serving them from django (which is only in dev) --- lms/envs/common.py | 20 +------------------- lms/envs/dev.py | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index 16472795e0..a2d9614109 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -266,24 +266,6 @@ STATICFILES_DIRS = [ COMMON_ROOT / "static", PROJECT_ROOT / "static", ] -if os.path.isdir(DATA_DIR): - # Add the full course repo if there is no static directory - STATICFILES_DIRS += [ - # TODO (cpennington): When courses are stored in a database, this - # should no longer be added to STATICFILES - (course_dir, DATA_DIR / course_dir) - for course_dir in os.listdir(DATA_DIR) - if (os.path.isdir(DATA_DIR / course_dir) and - not os.path.isdir(DATA_DIR / course_dir / 'static')) - ] - # Otherwise, add only the static directory from the course dir - STATICFILES_DIRS += [ - # TODO (cpennington): When courses are stored in a database, this - # should no longer be added to STATICFILES - (course_dir, DATA_DIR / course_dir / 'static') - for course_dir in os.listdir(DATA_DIR) - if (os.path.isdir(DATA_DIR / course_dir / 'static')) - ] # Locale/Internationalization TIME_ZONE = 'America/New_York' # http://en.wikipedia.org/wiki/List_of_tz_zones_by_name @@ -468,7 +450,7 @@ PIPELINE_JS = { 'source_filenames': sorted( set(rooted_glob(COMMON_ROOT / 'static', 'coffee/src/**/*.coffee') + rooted_glob(PROJECT_ROOT / 'static', 'coffee/src/**/*.coffee')) - - set(courseware_js + discussion_js + staff_grading_js + peer_grading_js) + set(courseware_js + discussion_js + staff_grading_js + peer_grading_js) ) + [ 'js/form.ext.js', 'js/my_courses_dropdown.js', diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 99ee9662ee..338a31f641 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -106,6 +106,27 @@ VIRTUAL_UNIVERSITIES = [] COMMENTS_SERVICE_KEY = "PUT_YOUR_API_KEY_HERE" +############################## Course static files ########################## +if os.path.isdir(DATA_DIR): + # Add the full course repo if there is no static directory + STATICFILES_DIRS += [ + # TODO (cpennington): When courses are stored in a database, this + # should no longer be added to STATICFILES + (course_dir, DATA_DIR / course_dir) + for course_dir in os.listdir(DATA_DIR) + if (os.path.isdir(DATA_DIR / course_dir) and + not os.path.isdir(DATA_DIR / course_dir / 'static')) + ] + # Otherwise, add only the static directory from the course dir + STATICFILES_DIRS += [ + # TODO (cpennington): When courses are stored in a database, this + # should no longer be added to STATICFILES + (course_dir, DATA_DIR / course_dir / 'static') + for course_dir in os.listdir(DATA_DIR) + if (os.path.isdir(DATA_DIR / course_dir / 'static')) + ] + + ################################# mitx revision string ##################### MITX_VERSION_STRING = os.popen('cd %s; git describe' % REPO_ROOT).read().strip() From 89f984c08be3747e27e2ea3a2345839c6ee2c24e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 29 Jan 2013 13:48:07 -0500 Subject: [PATCH 23/60] Prefix courseware urls with /static, since they are no longer served through collectstatic --- cms/djangoapps/contentstore/views.py | 2 +- lms/djangoapps/courseware/module_render.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 816ccab091..cc26510534 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -527,7 +527,7 @@ def load_preview_module(request, preview_id, descriptor, instance_state, shared_ module.get_html = replace_static_urls( module.get_html, - module.metadata.get('data_dir', module.location.course), + '/static/' + module.metadata.get('data_dir', module.location.course), course_namespace = Location([module.location.tag, module.location.org, module.location.course, None, None]) ) save_preview_state(request, preview_id, descriptor.location.url(), diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 7ed32c8597..a641f6e94c 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -280,7 +280,7 @@ def _get_module(user, request, descriptor, student_module_cache, course_id, module.get_html = replace_static_urls( _get_html, - module.metadata['data_dir'] if 'data_dir' in module.metadata else '', + '/static/' + module.metadata['data_dir'] if 'data_dir' in module.metadata else '', course_namespace = module.location._replace(category=None, name=None)) # Allow URLs of the form '/course/' refer to the root of multicourse directory From 95f2c9e275108f7045df67c33e45b88a9b37718e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 29 Jan 2013 14:20:01 -0500 Subject: [PATCH 24/60] Standardize how static urls are replaced by modules in their own html --- common/lib/xmodule/xmodule/capa_module.py | 4 ++-- lms/djangoapps/courseware/module_render.py | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index e4ab804f49..688737d883 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -355,7 +355,7 @@ class CapaModule(XModule): id=self.location.html_id(), ajax_url=self.system.ajax_url) + html + "
" # now do the substitutions which are filesystem based, e.g. '/static/' prefixes - return self.system.replace_urls(html, self.metadata['data_dir'], course_namespace=self.location) + return self.system.replace_urls(html) def handle_ajax(self, dispatch, get): ''' @@ -460,7 +460,7 @@ class CapaModule(XModule): new_answers = dict() for answer_id in answers: try: - new_answer = {answer_id: self.system.replace_urls(answers[answer_id], self.metadata['data_dir'], course_namespace=self.location)} + new_answer = {answer_id: self.system.replace_urls(answers[answer_id])} except TypeError: log.debug('Unable to perform URL substitution on answers[%s]: %s' % (answer_id, answers[answer_id])) new_answer = {answer_id: answers[answer_id]} diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index a641f6e94c..23dbc8c8a6 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -244,7 +244,11 @@ def _get_module(user, request, descriptor, student_module_cache, course_id, # TODO (cpennington): This should be removed when all html from # a module is coming through get_html and is therefore covered # by the replace_static_urls code below - replace_urls=replace_urls, + replace_urls=partial( + replace_urls, + staticfiles_prefix='/static/' + module.metadata.get('data_dir', ''), + course_namespace=module.location._replace(category=None, name=None), + ), node_path=settings.NODE_PATH, anonymous_student_id=unique_id_for_user(user), course_id=course_id, @@ -280,7 +284,7 @@ def _get_module(user, request, descriptor, student_module_cache, course_id, module.get_html = replace_static_urls( _get_html, - '/static/' + module.metadata['data_dir'] if 'data_dir' in module.metadata else '', + '/static/' + module.metadata.get('data_dir', ''), course_namespace = module.location._replace(category=None, name=None)) # Allow URLs of the form '/course/' refer to the root of multicourse directory From 9ae83338c627af75ed18d9894be265ba9a82c5b6 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 29 Jan 2013 14:20:21 -0500 Subject: [PATCH 25/60] Whitespace cleanup --- cms/djangoapps/contentstore/views.py | 136 +++++++++++----------- common/lib/xmodule/xmodule/capa_module.py | 6 +- 2 files changed, 71 insertions(+), 71 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index cc26510534..9331e97fec 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -132,7 +132,7 @@ def has_access(user, location, role=STAFF_ROLE_NAME): Return True if user allowed to access this piece of data Note that the CMS permissions model is with respect to courses There is a super-admin permissions if user.is_staff is set - Also, since we're unifying the user database between LMS and CAS, + Also, since we're unifying the user database between LMS and CAS, I'm presuming that the course instructor (formally known as admin) will not be in both INSTRUCTOR and STAFF groups, so we have to cascade our queries here as INSTRUCTOR has all the rights that STAFF do @@ -154,7 +154,7 @@ def course_index(request, org, course, name): org, course, name: Attributes of the Location for the item to edit """ location = ['i4x', org, course, 'course', name] - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() @@ -213,7 +213,7 @@ def edit_subsection(request, location): # remove all metadata from the generic dictionary that is presented in a more normalized UI - policy_metadata = dict((key,value) for key, value in item.metadata.iteritems() + policy_metadata = dict((key,value) for key, value in item.metadata.iteritems() if key not in ['display_name', 'start', 'due', 'format'] and key not in item.system_metadata_fields) can_view_live = False @@ -292,7 +292,7 @@ def edit_unit(request, location): containing_section = modulestore().get_item(containing_section_locs[0]) # cdodge hack. We're having trouble previewing drafts via jump_to redirect - # so let's generate the link url here + # so let's generate the link url here # need to figure out where this item is in the list of children as the preview will need this index =1 @@ -303,12 +303,12 @@ def edit_unit(request, location): preview_lms_link = '//{preview}{lms_base}/courses/{org}/{course}/{course_name}/courseware/{section}/{subsection}/{index}'.format( preview='preview.', - lms_base=settings.LMS_BASE, + lms_base=settings.LMS_BASE, org=course.location.org, - course=course.location.course, - course_name=course.location.name, - section=containing_section.location.name, - subsection=containing_subsection.location.name, + course=course.location.course, + course_name=course.location.name, + section=containing_section.location.name, + subsection=containing_subsection.location.name, index=index) unit_state = compute_unit_state(item) @@ -359,14 +359,14 @@ def assignment_type_update(request, org, course, category, name): location = Location(['i4x', org, course, category, name]) if not has_access(request.user, location): raise HttpResponseForbidden() - + if request.method == 'GET': - return HttpResponse(json.dumps(CourseGradingModel.get_section_grader_type(location)), + return HttpResponse(json.dumps(CourseGradingModel.get_section_grader_type(location)), mimetype="application/json") elif request.method == 'POST': # post or put, doesn't matter. - return HttpResponse(json.dumps(CourseGradingModel.update_section_grader_type(location, request.POST)), + return HttpResponse(json.dumps(CourseGradingModel.update_section_grader_type(location, request.POST)), mimetype="application/json") - + def user_author_string(user): '''Get an author string for commits by this user. Format: @@ -511,20 +511,20 @@ def load_preview_module(request, preview_id, descriptor, instance_state, shared_ error_msg=exc_info_to_str(sys.exc_info()) ).xmodule_constructor(system)(None, None) - # cdodge: Special case + # cdodge: Special case if module.location.category == 'static_tab': module.get_html = wrap_xmodule( module.get_html, module, "xmodule_tab_display.html", ) - else: + else: module.get_html = wrap_xmodule( module.get_html, module, "xmodule_display.html", ) - + module.get_html = replace_static_urls( module.get_html, '/static/' + module.metadata.get('data_dir', module.location.course), @@ -555,7 +555,7 @@ def _xmodule_recurse(item, action): _xmodule_recurse(child, action) action(item) - + @login_required @expect_json @@ -590,7 +590,7 @@ def delete_item(request): # delete_item on a vertical tries to delete the draft version leaving the # requested delete to never occur if item.location.revision is None and item.location.category=='vertical' and delete_all_versions: - modulestore('direct').delete_item(item.location) + modulestore('direct').delete_item(item.location) return HttpResponse() @@ -609,7 +609,7 @@ def save_item(request): if request.POST.get('data') is not None: data = request.POST['data'] store.update_item(item_location, data) - + # cdodge: note calling request.POST.get('children') will return None if children is an empty array # so it lead to a bug whereby the last component to be deleted in the UI was not actually # deleting the children object from the children collection @@ -699,7 +699,7 @@ def unpublish_unit(request): def clone_item(request): parent_location = Location(request.POST['parent_location']) template = Location(request.POST['template']) - + display_name = request.POST.get('display_name') if not has_access(request.user, parent_location): @@ -739,9 +739,9 @@ def upload_asset(request, org, course, coursename): location = ['i4x', org, course, 'course', coursename] if not has_access(request.user, location): return HttpResponseForbidden() - + # Does the course actually exist?!? Get anything from it to prove its existance - + try: item = modulestore().get_item(location) except: @@ -775,9 +775,9 @@ def upload_asset(request, org, course, coursename): # readback the saved content - we need the database timestamp readback = contentstore().find(content.location) - - response_payload = {'displayname' : content.name, - 'uploadDate' : get_date_display(readback.last_modified_at), + + response_payload = {'displayname' : content.name, + 'uploadDate' : get_date_display(readback.last_modified_at), 'url' : StaticContent.get_url_path_from_location(content.location), 'thumb_url' : StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_content is not None else None, 'msg' : 'Upload completed' @@ -793,7 +793,7 @@ This view will return all CMS users who are editors for the specified course @login_required @ensure_csrf_cookie def manage_users(request, location): - + # check that logged in user has permissions to this item if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME) and not has_access(request.user, location, role=STAFF_ROLE_NAME): raise PermissionDenied() @@ -809,7 +809,7 @@ def manage_users(request, location): 'allow_actions' : has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME), 'request_user_id' : request.user.id }) - + def create_json_response(errmsg = None): if errmsg is not None: @@ -831,13 +831,13 @@ def add_user(request, location): if email=='': return create_json_response('Please specify an email address.') - + # check that logged in user has admin permissions to this course if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME): raise PermissionDenied() - + user = get_user_by_email(email) - + # user doesn't exist?!? Return error. if user is None: return create_json_response('Could not find user by email address \'{0}\'.'.format(email)) @@ -860,7 +860,7 @@ the specified course @ensure_csrf_cookie def remove_user(request, location): email = request.POST["email"] - + # check that logged in user has admin permissions on this course if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME): raise PermissionDenied() @@ -887,7 +887,7 @@ def landing(request, org, course, coursename): def static_pages(request, org, course, coursename): location = ['i4x', org, course, 'course', coursename] - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() @@ -906,7 +906,7 @@ def edit_static(request, org, course, coursename): @login_required @ensure_csrf_cookie def edit_tabs(request, org, course, coursename): - location = ['i4x', org, course, 'course', coursename] + location = ['i4x', org, course, 'course', coursename] course_item = modulestore().get_item(location) static_tabs_loc = Location('i4x', org, course, 'static_tab', None) @@ -928,7 +928,7 @@ def edit_tabs(request, org, course, coursename): return render_to_response('edit-tabs.html', { 'active_tab': 'pages', - 'context_course':course_item, + 'context_course':course_item, 'components': components }) @@ -949,13 +949,13 @@ def course_info(request, org, course, name, provided_id=None): org, course, name: Attributes of the Location for the item to edit """ location = ['i4x', org, course, 'course', name] - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() - + course_module = modulestore().get_item(location) - + # get current updates location = ['i4x', org, course, 'course_info', "updates"] @@ -966,7 +966,7 @@ def course_info(request, org, course, name, provided_id=None): 'course_updates' : json.dumps(get_course_updates(location)), 'handouts_location': Location(['i4x', org, course, 'course_info', 'handouts']).url() }) - + @expect_json @login_required @ensure_csrf_cookie @@ -980,7 +980,7 @@ def course_info_updates(request, org, course, provided_id=None): # ??? No way to check for access permission afaik # get current updates location = ['i4x', org, course, 'course_info', "updates"] - + # Hmmm, provided_id is coming as empty string on create whereas I believe it used to be None :-( # Possibly due to my removing the seemingly redundant pattern in urls.py if provided_id == '': @@ -995,7 +995,7 @@ def course_info_updates(request, org, course, provided_id=None): real_method = request.META['HTTP_X_HTTP_METHOD_OVERRIDE'] else: real_method = request.method - + if request.method == 'GET': return HttpResponse(json.dumps(get_course_updates(location)), mimetype="application/json") elif real_method == 'DELETE': # coming as POST need to pull from Request Header X-HTTP-Method-Override DELETE @@ -1012,7 +1012,7 @@ def course_info_updates(request, org, course, provided_id=None): @ensure_csrf_cookie def module_info(request, module_location): location = Location(module_location) - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() @@ -1025,10 +1025,10 @@ def module_info(request, module_location): rewrite_static_links = request.GET.get('rewrite_url_links','True') in ['True', 'true'] logging.debug('rewrite_static_links = {0} {1}'.format(request.GET.get('rewrite_url_links','False'), rewrite_static_links)) - + # check that logged in user has permissions to this item if not has_access(request.user, location): - raise PermissionDenied() + raise PermissionDenied() if real_method == 'GET': return HttpResponse(json.dumps(get_module_info(get_modulestore(location), location, rewrite_static_links=rewrite_static_links)), mimetype="application/json") @@ -1046,20 +1046,20 @@ def get_course_settings(request, org, course, name): org, course, name: Attributes of the Location for the item to edit """ location = ['i4x', org, course, 'course', name] - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() - + course_module = modulestore().get_item(location) course_details = CourseDetails.fetch(location) - + return render_to_response('settings.html', { - 'active_tab': 'settings', + 'active_tab': 'settings', 'context_course': course_module, 'course_details' : json.dumps(course_details, cls=CourseSettingsEncoder) }) - + @expect_json @login_required @ensure_csrf_cookie @@ -1082,13 +1082,13 @@ def course_settings_updates(request, org, course, name, section): elif section == 'grading': manager = CourseGradingModel else: return - + if request.method == 'GET': # Cannot just do a get w/o knowing the course name :-( - return HttpResponse(json.dumps(manager.fetch(Location(['i4x', org, course, 'course',name])), cls=CourseSettingsEncoder), + return HttpResponse(json.dumps(manager.fetch(Location(['i4x', org, course, 'course',name])), cls=CourseSettingsEncoder), mimetype="application/json") elif request.method == 'POST': # post or put, doesn't matter. - return HttpResponse(json.dumps(manager.update_from_json(request.POST), cls=CourseSettingsEncoder), + return HttpResponse(json.dumps(manager.update_from_json(request.POST), cls=CourseSettingsEncoder), mimetype="application/json") @expect_json @@ -1101,7 +1101,7 @@ def course_grader_updates(request, org, course, name, grader_index=None): org, course: Attributes of the Location for the item to edit """ - + location = ['i4x', org, course, 'course', name] # check that logged in user has permissions to this item @@ -1112,13 +1112,13 @@ def course_grader_updates(request, org, course, name, grader_index=None): real_method = request.META['HTTP_X_HTTP_METHOD_OVERRIDE'] else: real_method = request.method - + if real_method == 'GET': # Cannot just do a get w/o knowing the course name :-( - return HttpResponse(json.dumps(CourseGradingModel.fetch_grader(Location(['i4x', org, course, 'course',name]), grader_index)), + return HttpResponse(json.dumps(CourseGradingModel.fetch_grader(Location(['i4x', org, course, 'course',name]), grader_index)), mimetype="application/json") elif real_method == "DELETE": - # ??? Shoudl this return anything? Perhaps success fail? + # ??? Shoudl this return anything? Perhaps success fail? CourseGradingModel.delete_grader(Location(['i4x', org, course, 'course',name]), grader_index) return HttpResponse() elif request.method == 'POST': # post or put, doesn't matter. @@ -1135,7 +1135,7 @@ def asset_index(request, org, course, name): org, course, name: Attributes of the Location for the item to edit """ location = ['i4x', org, course, 'course', name] - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() @@ -1148,7 +1148,7 @@ def asset_index(request, org, course, name): }) course_module = modulestore().get_item(location) - + course_reference = StaticContent.compute_location(org, course, name) assets = contentstore().get_all_content_for_course(course_reference) @@ -1162,15 +1162,15 @@ def asset_index(request, org, course, name): display_info = {} display_info['displayname'] = asset['displayname'] display_info['uploadDate'] = get_date_display(asset['uploadDate']) - + asset_location = StaticContent.compute_location(id['org'], id['course'], id['name']) display_info['url'] = StaticContent.get_url_path_from_location(asset_location) - + # note, due to the schema change we may not have a 'thumbnail_location' in the result set _thumbnail_location = asset.get('thumbnail_location', None) thumbnail_location = Location(_thumbnail_location) if _thumbnail_location is not None else None display_info['thumb_url'] = StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_location is not None else None - + asset_display.append(display_info) return render_to_response('asset_index.html', { @@ -1189,9 +1189,9 @@ def edge(request): @expect_json def create_new_course(request): template = Location(request.POST['template']) - org = request.POST.get('org') - number = request.POST.get('number') - display_name = request.POST.get('display_name') + org = request.POST.get('org') + number = request.POST.get('number') + display_name = request.POST.get('display_name') try: dest_location = Location('i4x', org, number, 'course', Location.clean(display_name)) @@ -1237,13 +1237,13 @@ def initialize_course_tabs(course): # at least a list populated with the minimal times # @TODO: I don't like the fact that the presentation tier is away of these data related constraints, let's find a better # place for this. Also rather than using a simple list of dictionaries a nice class model would be helpful here - course.tabs = [{"type": "courseware"}, - {"type": "course_info", "name": "Course Info"}, + course.tabs = [{"type": "courseware"}, + {"type": "course_info", "name": "Course Info"}, {"type": "discussion", "name": "Discussion"}, {"type": "wiki", "name": "Wiki"}, {"type": "progress", "name": "Progress"}] - modulestore('direct').update_metadata(course.location.url(), course.own_metadata) + modulestore('direct').update_metadata(course.location.url(), course.own_metadata) @ensure_csrf_cookie @login_required @@ -1337,7 +1337,7 @@ def generate_export_course(request, org, course, name): root_dir = path(mkdtemp()) # export out to a tempdir - + logging.debug('root = {0}'.format(root_dir)) export_to_xml(modulestore('direct'), contentstore(), loc, root_dir, name) @@ -1349,7 +1349,7 @@ def generate_export_course(request, org, course, name): tf.close() # remove temp dir - shutil.rmtree(root_dir/name) + shutil.rmtree(root_dir/name) wrapper = FileWrapper(export_file) response = HttpResponse(wrapper, content_type='application/x-tgz') diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 688737d883..3947945beb 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -668,18 +668,18 @@ class CapaDescriptor(RawDescriptor): # TODO (vshnayder): do problems have any other metadata? Do they # actually use type and points? metadata_attributes = RawDescriptor.metadata_attributes + ('type', 'points') - + def get_context(self): _context = RawDescriptor.get_context(self) _context.update({'markdown': self.metadata.get('markdown', '')}) return _context - + @property def editable_metadata_fields(self): """Remove metadata from the editable fields since it has its own editor""" subset = super(CapaDescriptor,self).editable_metadata_fields if 'markdown' in subset: - subset.remove('markdown') + subset.remove('markdown') return subset From e773b8e7b126718fbc8040148ef2455790f913c4 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 29 Jan 2013 14:22:39 -0500 Subject: [PATCH 26/60] Fix import error --- lms/djangoapps/courseware/module_render.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 23dbc8c8a6..064fdf1210 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -3,6 +3,8 @@ import logging import pyparsing import sys +from functools import partial + from django.conf import settings from django.contrib.auth.models import User from django.core.urlresolvers import reverse From 07487a8d8f5124d5cfaa16b58b488769f7c42ac2 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 29 Jan 2013 14:27:27 -0500 Subject: [PATCH 27/60] Use descriptor when setting up replace_urls, as it's been created at that point --- lms/djangoapps/courseware/module_render.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 064fdf1210..22d95ef8a2 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -248,8 +248,8 @@ def _get_module(user, request, descriptor, student_module_cache, course_id, # by the replace_static_urls code below replace_urls=partial( replace_urls, - staticfiles_prefix='/static/' + module.metadata.get('data_dir', ''), - course_namespace=module.location._replace(category=None, name=None), + staticfiles_prefix='/static/' + descriptor.metadata.get('data_dir', ''), + course_namespace=descriptor.location._replace(category=None, name=None), ), node_path=settings.NODE_PATH, anonymous_student_id=unique_id_for_user(user), From 2854d7d06695a9ead9920f90678cf3807c64644f Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 29 Jan 2013 16:21:46 -0500 Subject: [PATCH 28/60] Point course_image.jpg at the correct place in the /static url space --- lms/djangoapps/courseware/courses.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 89a1496eca..03d5a89c64 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -83,13 +83,12 @@ def get_opt_course_with_access(user, course_id, action): return None return get_course_with_access(user, course_id, action) - + def course_image_url(course): """Try to look up the image url for the course. If it's not found, log an error and return the dead link""" if isinstance(modulestore(), XMLModuleStore): - path = course.metadata['data_dir'] + "/images/course_image.jpg" - return try_staticfiles_lookup(path) + return '/static/' + course.metadata['data_dir'] + "/images/course_image.jpg" else: loc = course.location._replace(tag='c4x', category='asset', name='images_course_image.jpg') path = StaticContent.get_url_path_from_location(loc) From 3c1c61fb19ea96c1ed51e9b274c29cb5f0252c27 Mon Sep 17 00:00:00 2001 From: cahrens Date: Wed, 30 Jan 2013 09:23:07 -0500 Subject: [PATCH 29/60] Fixes for https://edx.lighthouseapp.com/projects/102637/tickets/144 (progress tab not updating). --- lms/djangoapps/courseware/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 8c529a8585..5d65d7c632 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -233,7 +233,7 @@ def index(request, course_id, chapter=None, section=None, # Specifically asked-for section doesn't exist raise Http404 - # Load all descendants of the section, because we're going to display it's + # Load all descendants of the section, because we're going to display its # html, which in general will need all of its children section_module_cache = StudentModuleCache.cache_for_descriptor_descendents( course.id, request.user, section_descriptor, depth=None) From ce8a48405b958575add205fcc73d77ca5a4ace8a Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Wed, 30 Jan 2013 09:31:20 -0500 Subject: [PATCH 30/60] Clean up some JS errors showing up in the xmodule jasmine tests --- common/lib/xmodule/jasmine_test_runner.html.erb | 1 - common/lib/xmodule/xmodule/js/spec/helper.coffee | 1 - 2 files changed, 2 deletions(-) diff --git a/common/lib/xmodule/jasmine_test_runner.html.erb b/common/lib/xmodule/jasmine_test_runner.html.erb index fae6c14cbe..7b078daedd 100644 --- a/common/lib/xmodule/jasmine_test_runner.html.erb +++ b/common/lib/xmodule/jasmine_test_runner.html.erb @@ -14,7 +14,6 @@ - diff --git a/common/lib/xmodule/xmodule/js/spec/helper.coffee b/common/lib/xmodule/xmodule/js/spec/helper.coffee index dc01241861..fbc89f7bd9 100644 --- a/common/lib/xmodule/xmodule/js/spec/helper.coffee +++ b/common/lib/xmodule/xmodule/js/spec/helper.coffee @@ -64,7 +64,6 @@ jasmine.stubVideoPlayer = (context, enableParts, createPlayer=true) -> if createPlayer return new VideoPlayer(video: context.video) -spyOn(window, 'onunload') # Stub jQuery.cookie $.cookie = jasmine.createSpy('jQuery.cookie').andReturn '1.0' From 4df62d968d9ad1f8b2f364e958d264cf469aa7ad Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Wed, 30 Jan 2013 09:31:42 -0500 Subject: [PATCH 31/60] Fix up new jasmine tests --- .../js/spec/combinedopenended/display_spec.coffee | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee b/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee index 3ccf504948..02aaa50103 100644 --- a/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee @@ -1,16 +1,15 @@ -xdescribe 'CombinedOpenEnded', -> +describe 'CombinedOpenEnded', -> beforeEach -> spyOn Logger, 'log' # load up some fixtures loadFixtures 'combined-open-ended.html' @element = $('.combined-open-ended') - describe 'constructor', -> - beforeEach -> - @combined = new CombinedOpenEnded @element - it 'set the element', -> - except(@combined.element).not.toEqual @element + describe 'constructor', -> + beforeEach -> + @combined = new CombinedOpenEnded @element + it 'set the element', -> + expect(@combined.element).toEqual @element - #it 'initialize the ajax url, state, and task count', -> From ab4fd03dd8c5b42afcaa0258e27d51245d613671 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 30 Jan 2013 09:43:27 -0500 Subject: [PATCH 32/60] Catch any and all save errors --- .../js/models/settings/course_details.js | 12 +++++- .../js/views/settings/main_settings_view.js | 38 +++++++++++++++---- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/cms/static/js/models/settings/course_details.js b/cms/static/js/models/settings/course_details.js index ab80179142..61e31133fd 100644 --- a/cms/static/js/models/settings/course_details.js +++ b/cms/static/js/models/settings/course_details.js @@ -68,10 +68,18 @@ CMS.Models.Settings.CourseDetails = Backbone.Model.extend({ save_videosource: function(newsource) { // newsource either is