From c2e5c607136e93a8c63d1aa073fe0ef1756a24f3 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 13 Jun 2013 15:34:00 -0400 Subject: [PATCH 001/112] Remove correct/incorrect message, and auto-add in needed markdown for studio --- .../lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee | 4 ++++ lms/templates/combinedopenended/openended/open_ended.html | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee b/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee index 1b7f9bb4fb..2f58e86cb2 100644 --- a/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee +++ b/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee @@ -50,6 +50,10 @@ Write a persuasive essay to a newspaper reflecting your vies on censorship in li mode: null }) @setCurrentEditor(@markdown_editor) + selection = @markdown_editor.getSelection() + #Auto-add in the needed template if it isn't already in there. + if(@markdown_editor.getValue() == "") + @markdown_editor.setValue(OpenEndedMarkdownEditingDescriptor.promptTemplate + "\n" + OpenEndedMarkdownEditingDescriptor.rubricTemplate + "\n" + OpenEndedMarkdownEditingDescriptor.tasksTemplate) # Add listeners for toolbar buttons (only present for markdown editor) @element.on('click', '.xml-tab', @onShowXMLButton) @element.on('click', '.format-buttons a', @onToolbarButton) diff --git a/lms/templates/combinedopenended/openended/open_ended.html b/lms/templates/combinedopenended/openended/open_ended.html index 9fb136cee6..909ef15838 100644 --- a/lms/templates/combinedopenended/openended/open_ended.html +++ b/lms/templates/combinedopenended/openended/open_ended.html @@ -10,10 +10,6 @@
% if state == 'initial': Unanswered - % elif state in ['done', 'post_assessment'] and correct == 'correct': -

Correct

- % elif state in ['done', 'post_assessment'] and correct == 'incorrect': -

Incorrect.

% elif state == 'assessing': Submitted for grading. % if eta_message is not None: From cd49e1bb686f747969d13e875111f448cf5b71bb Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 13 Jun 2013 16:00:28 -0400 Subject: [PATCH 002/112] Check to see if a location is valid before displaying it for instructor grading --- .../open_ended_grading/staff_grading_service.py | 11 ++++++++++- lms/djangoapps/open_ended_grading/utils.py | 16 ++++++++++++++++ lms/djangoapps/open_ended_grading/views.py | 2 +- 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 lms/djangoapps/open_ended_grading/utils.py diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index 2c611b4481..a2d905be0b 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -15,6 +15,7 @@ from xmodule.course_module import CourseDescriptor from student.models import unique_id_for_user from xmodule.x_module import ModuleSystem from mitxmako.shortcuts import render_to_string +from utils import does_location_exist log = logging.getLogger(__name__) @@ -240,7 +241,6 @@ def get_next(request, course_id): return HttpResponse(_get_next(course_id, grader_id, location), mimetype="application/json") - def get_problem_list(request, course_id): """ Get all the problems for the given course id @@ -266,6 +266,15 @@ def get_problem_list(request, course_id): _check_access(request.user, course_id) try: response = staff_grading_service().get_problem_list(course_id, unique_id_for_user(request.user)) + response = json.loads(response) + problem_list = response['problem_list'] + valid_problem_list = [] + for i in xrange(0,len(problem_list)): + if does_location_exist(course_id, problem_list[i]['location']): + valid_problem_list.append(problem_list[i]) + response['problem_list'] = valid_problem_list + response = json.dumps(response) + return HttpResponse(response, mimetype="application/json") except GradingServiceError: diff --git a/lms/djangoapps/open_ended_grading/utils.py b/lms/djangoapps/open_ended_grading/utils.py new file mode 100644 index 0000000000..7634977397 --- /dev/null +++ b/lms/djangoapps/open_ended_grading/utils.py @@ -0,0 +1,16 @@ +from xmodule.modulestore import search +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import ItemNotFoundError + +def does_location_exist(course_id, location): + """ + Checks to see if a valid module exists at a given location (ie has not been deleted) + course_id - string course id + location - string location + """ + try: + search.path_to_location(modulestore(), course_id, location) + return True + except ItemNotFoundError: + #If the problem cannot be found at the location received from the grading controller server, it has been deleted by the course author. + return False diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index a914e434a9..797c90f9a4 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -179,6 +179,7 @@ def student_problem_list(request, course_id): error_text = "" problem_list = [] base_course_url = reverse('courses') + list_to_remove = [] try: #Get list of all open ended problems that the grading server knows about @@ -192,7 +193,6 @@ def student_problem_list(request, course_id): problem_list = problem_list_dict['problem_list'] #A list of problems to remove (problems that can't be found in the course) - list_to_remove = [] for i in xrange(0, len(problem_list)): try: #Try to load each problem in the courseware to get links to them From 9b14ea790c30f25792c05bf6901c3ae44cc4c603 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 27 Jun 2013 16:20:58 -0400 Subject: [PATCH 003/112] Properly increment student attempts --- .../open_ended_grading_classes/combined_open_ended_modulev1.py | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index 1fe62035e6..c99d0915cb 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -653,6 +653,7 @@ class CombinedOpenEndedV1Module(): ).format(self.student_attempts, self.attempts) } self.state = self.INITIAL + self.student_attempts +=1 self.ready_to_reset = False for i in xrange(0, len(self.task_xml)): self.current_task_number = i From 8a6c8b5ab0ac79d5e458e95fc743351dd0c6f8cc Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 28 Jun 2013 14:10:17 -0400 Subject: [PATCH 004/112] Add test for incrementing student attempts --- .../combined_open_ended_modulev1.py | 4 +- .../xmodule/tests/test_combined_open_ended.py | 51 +++++++++++++++++++ .../SampleQuestion1Attempt.xml | 24 +++++++++ .../test/data/open_ended/course/2012_Fall.xml | 1 + 4 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 common/test/data/open_ended/combinedopenended/SampleQuestion1Attempt.xml diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index c99d0915cb..62c9417660 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -643,7 +643,8 @@ class CombinedOpenEndedV1Module(): if not self.ready_to_reset: return self.out_of_sync_error(data) - if self.student_attempts > self.attempts: + self.student_attempts +=1 + if self.student_attempts >= self.attempts: return { 'success': False, # This is a student_facing_error @@ -653,7 +654,6 @@ class CombinedOpenEndedV1Module(): ).format(self.student_attempts, self.attempts) } self.state = self.INITIAL - self.student_attempts +=1 self.ready_to_reset = False for i in xrange(0, len(self.task_xml)): self.current_task_number = i 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 e1f8d135de..dc0023f80d 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -629,3 +629,54 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): #reset the problem module.handle_ajax("reset", {}) self.assertEqual(module.state, "initial") + +class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): + """ + Test the student flow in the combined open ended xmodule + """ + problem_location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestion1Attempt"]) + answer = "blah blah" + assessment = [0, 1] + hint = "blah" + + def setUp(self): + self.test_system = get_test_system() + self.test_system.xqueue['interface'] = Mock( + send_to_queue=Mock(side_effect=[1, "queued"]) + ) + self.setup_modulestore(COURSE) + + def test_reset_fail(self): + """ + Test the flow of the module if we complete the self assessment step and then reset + @return: + """ + assessment = [0, 1] + module = self.get_module_from_location(self.problem_location, COURSE) + + #Simulate a student saving an answer + module.handle_ajax("save_answer", {"student_answer": self.answer}) + status = module.handle_ajax("get_status", {}) + self.assertTrue(isinstance(status, basestring)) + + #Mock a student submitting an assessment + assessment_dict = MockQueryDict() + assessment_dict.update({'assessment': sum(assessment), 'score_list[]': assessment}) + module.handle_ajax("save_assessment", assessment_dict) + task_one_json = json.loads(module.task_states[0]) + self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment) + status = module.handle_ajax("get_status", {}) + self.assertTrue(isinstance(status, basestring)) + + #Move to the next step in the problem + module.handle_ajax("next_problem", {}) + self.assertEqual(module.current_task_number, 0) + + html = module.get_html() + self.assertTrue(isinstance(html, basestring)) + + rubric = module.handle_ajax("get_combined_rubric", {}) + self.assertTrue(isinstance(rubric, basestring)) + self.assertEqual(module.state, "done") + reset_data = json.loads(module.handle_ajax("reset", {})) + self.assertEqual(reset_data['success'], False) diff --git a/common/test/data/open_ended/combinedopenended/SampleQuestion1Attempt.xml b/common/test/data/open_ended/combinedopenended/SampleQuestion1Attempt.xml new file mode 100644 index 0000000000..9bfabca191 --- /dev/null +++ b/common/test/data/open_ended/combinedopenended/SampleQuestion1Attempt.xml @@ -0,0 +1,24 @@ + + + + + Writing Applications + + + + + Language Conventions + + + + + + +

Censorship in the Libraries

+

"All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us." --Katherine Paterson, Author

+

Write a persuasive essay to a newspaper reflecting your vies on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading.

+ + + + + \ No newline at end of file diff --git a/common/test/data/open_ended/course/2012_Fall.xml b/common/test/data/open_ended/course/2012_Fall.xml index 32c810174b..609d12f94c 100644 --- a/common/test/data/open_ended/course/2012_Fall.xml +++ b/common/test/data/open_ended/course/2012_Fall.xml @@ -1,6 +1,7 @@ + From b38a36d44a5600c44da9840b59eb2dbef08a6bdf Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 28 Jun 2013 14:38:13 -0400 Subject: [PATCH 005/112] Fix test --- .../lib/xmodule/xmodule/tests/test_combined_open_ended.py | 8 ++++++-- .../open_ended_grading/staff_grading_service.py | 5 +++++ 2 files changed, 11 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 dc0023f80d..af1b6aa12b 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -630,9 +630,9 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): module.handle_ajax("reset", {}) self.assertEqual(module.state, "initial") -class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): +class OpenEndedModuleXmlAttemptTest(unittest.TestCase, DummyModulestore): """ - Test the student flow in the combined open ended xmodule + Test if student is able to reset the problem """ problem_location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestion1Attempt"]) answer = "blah blah" @@ -649,6 +649,7 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): def test_reset_fail(self): """ Test the flow of the module if we complete the self assessment step and then reset + Since the problem only allows one attempt, should fail. @return: """ assessment = [0, 1] @@ -675,8 +676,11 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): html = module.get_html() self.assertTrue(isinstance(html, basestring)) + #Module should now be done rubric = module.handle_ajax("get_combined_rubric", {}) self.assertTrue(isinstance(rubric, basestring)) self.assertEqual(module.state, "done") + + #Try to reset, should fail because only 1 attempt is allowed reset_data = json.loads(module.handle_ajax("reset", {})) self.assertEqual(reset_data['success'], False) diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index eda78d0b09..a6a2847bc1 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -270,6 +270,11 @@ def get_problem_list(request, course_id): problem_list = response['problem_list'] valid_problem_list = [] for i in xrange(0,len(problem_list)): + #Needed to ensure that the 'location' key can be accessed + try: + problem_list[i] = json.loads(problem_list[i]) + except Exception: + pass if does_location_exist(course_id, problem_list[i]['location']): valid_problem_list.append(problem_list[i]) response['problem_list'] = valid_problem_list From 8930e753e4c225a16e862825aac84cb15469fb8d Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 28 Jun 2013 14:59:39 -0400 Subject: [PATCH 006/112] Set default weights --- .../xmodule/combined_open_ended_module.py | 70 ++++++++++++++----- .../xmodule/xmodule/peer_grading_module.py | 29 +++++--- 2 files changed, 75 insertions(+), 24 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 52d98f032e..fdc85369a1 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -51,47 +51,85 @@ class CombinedOpenEndedFields(object): display_name = String( display_name="Display Name", help="This name appears in the horizontal navigation at the top of the page.", - default="Open Ended Grading", scope=Scope.settings + default="Open Ended Grading", + scope=Scope.settings + ) + current_task_number = Integer( + help="Current task that the student is on.", + default=0, + scope=Scope.user_state + ) + task_states = List( + help="List of state dictionaries of each task within this module.", + scope=Scope.user_state + ) + state = String( + help="Which step within the current task that the student is on.", + default="initial", + scope=Scope.user_state + ) + student_attempts = Integer( + help="Number of attempts taken by the student on this problem", + default=0, + scope=Scope.user_state ) - current_task_number = Integer(help="Current task that the student is on.", default=0, scope=Scope.user_state) - task_states = List(help="List of state dictionaries of each task within this module.", scope=Scope.user_state) - state = String(help="Which step within the current task that the student is on.", default="initial", - scope=Scope.user_state) - student_attempts = Integer(help="Number of attempts taken by the student on this problem", default=0, - scope=Scope.user_state) ready_to_reset = Boolean( - help="If the problem is ready to be reset or not.", default=False, + help="If the problem is ready to be reset or not.", + default=False, scope=Scope.user_state ) attempts = Integer( display_name="Maximum Attempts", - help="The number of times the student can try to answer this problem.", default=1, + help="The number of times the student can try to answer this problem.", + default=1, scope=Scope.settings, values={"min" : 1 } ) - is_graded = Boolean(display_name="Graded", help="Whether or not the problem is graded.", default=False, scope=Scope.settings) + is_graded = Boolean( + display_name="Graded", + help="Whether or not the problem is graded.", + default=False, + scope=Scope.settings + ) accept_file_upload = Boolean( display_name="Allow File Uploads", - help="Whether or not the student can submit files as a response.", default=False, scope=Scope.settings + help="Whether or not the student can submit files as a response.", + default=False, + scope=Scope.settings ) skip_spelling_checks = Boolean( display_name="Disable Quality Filter", help="If False, the Quality Filter is enabled and submissions with poor spelling, short length, or poor grammar will not be peer reviewed.", default=False, scope=Scope.settings ) - due = Date(help="Date that this problem is due by", default=None, scope=Scope.settings) + due = Date( + help="Date that this problem is due by", + default=None, + scope=Scope.settings + ) graceperiod = String( help="Amount of time after the due date that submissions will be accepted", default=None, scope=Scope.settings ) - version = VersionInteger(help="Current version number", default=DEFAULT_VERSION, scope=Scope.settings) - data = String(help="XML data for the problem", scope=Scope.content) + version = VersionInteger( + help="Current version number", + default=DEFAULT_VERSION, + scope=Scope.settings + ) + data = String( + help="XML data for the problem", + scope=Scope.content + ) weight = Float( display_name="Problem Weight", help="Defines the number of points each problem is worth. If the value is not set, each problem is worth one point.", - scope=Scope.settings, values={"min" : 0 , "step": ".1"} + scope=Scope.settings, values={"min" : 0 , "step": ".1"}, + default=1 + ) + markdown = String( + help="Markdown source of this module", + scope=Scope.settings ) - markdown = String(help="Markdown source of this module", scope=Scope.settings) class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule): diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index 7df444a892..f92971796c 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -32,23 +32,35 @@ class PeerGradingFields(object): display_name="Show Single Problem", help='When True, only the single problem specified by "Link to Problem Location" is shown. ' 'When False, a panel is displayed with all problems available for peer grading.', - default=USE_FOR_SINGLE_LOCATION, scope=Scope.settings + default=USE_FOR_SINGLE_LOCATION, + scope=Scope.settings ) link_to_location = String( display_name="Link to Problem Location", help='The location of the problem being graded. Only used when "Show Single Problem" is True.', - default=LINK_TO_LOCATION, scope=Scope.settings + default=LINK_TO_LOCATION, + scope=Scope.settings ) is_graded = Boolean( display_name="Graded", help='Defines whether the student gets credit for grading this problem. Only used when "Show Single Problem" is True.', - default=IS_GRADED, scope=Scope.settings + default=IS_GRADED, + scope=Scope.settings + ) + due_date = Date( + help="Due date that should be displayed.", + default=None, + scope=Scope.settings) + grace_period_string = String( + help="Amount of grace to give on the due date.", + default=None, + scope=Scope.settings ) - due_date = Date(help="Due date that should be displayed.", default=None, scope=Scope.settings) - grace_period_string = String(help="Amount of grace to give on the due date.", default=None, scope=Scope.settings) max_grade = Integer( - help="The maximum grade that a student can receive for this problem.", default=MAX_SCORE, - scope=Scope.settings, values={"min": 0} + help="The maximum grade that a student can receive for this problem.", + default=MAX_SCORE, + scope=Scope.settings, + values={"min": 0} ) student_data_for_location = Dict( help="Student data for a given peer grading problem.", @@ -57,7 +69,8 @@ class PeerGradingFields(object): weight = Float( display_name="Problem Weight", help="Defines the number of points each problem is worth. If the value is not set, each problem is worth one point.", - scope=Scope.settings, values={"min": 0, "step": ".1"} + scope=Scope.settings, values={"min": 0, "step": ".1"}, + default=1 ) From 2246944594cf2199e4c57fe371755011b4de5be8 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 28 Jun 2013 15:09:19 -0400 Subject: [PATCH 007/112] Set a default weight --- .../xmodule/combined_open_ended_module.py | 9 ++++++--- .../combined_open_ended_modulev1.py | 11 ++++++++--- .../lib/xmodule/xmodule/peer_grading_module.py | 16 ++++++++++------ 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index fdc85369a1..736b82e21d 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -82,7 +82,8 @@ class CombinedOpenEndedFields(object): display_name="Maximum Attempts", help="The number of times the student can try to answer this problem.", default=1, - scope=Scope.settings, values={"min" : 1 } + scope=Scope.settings, + values={"min" : 1 } ) is_graded = Boolean( display_name="Graded", @@ -99,7 +100,8 @@ class CombinedOpenEndedFields(object): skip_spelling_checks = Boolean( display_name="Disable Quality Filter", help="If False, the Quality Filter is enabled and submissions with poor spelling, short length, or poor grammar will not be peer reviewed.", - default=False, scope=Scope.settings + default=False, + scope=Scope.settings ) due = Date( help="Date that this problem is due by", @@ -123,7 +125,8 @@ class CombinedOpenEndedFields(object): weight = Float( display_name="Problem Weight", help="Defines the number of points each problem is worth. If the value is not set, each problem is worth one point.", - scope=Scope.settings, values={"min" : 0 , "step": ".1"}, + scope=Scope.settings, + values={"min" : 0 , "step": ".1"}, default=1 ) markdown = String( diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index 62c9417660..926bd780f7 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -727,7 +727,12 @@ class CombinedOpenEndedV1Module(): """ max_score = None score = None - if self.is_scored and self.weight is not None: + + #The old default was None, so set to 1 if it is the old default weight + weight = self.weight + if weight is None: + weight = 1 + if self.is_scored: # Finds the maximum score of all student attempts and keeps it. score_mat = [] for i in xrange(0, len(self.task_states)): @@ -740,7 +745,7 @@ class CombinedOpenEndedV1Module(): for z in xrange(0, len(score)): if score[z] is None: score[z] = 0 - score[z] *= float(self.weight) + score[z] *= float(weight) score_mat.append(score) if len(score_mat) > 0: @@ -754,7 +759,7 @@ class CombinedOpenEndedV1Module(): if max_score is not None: # Weight the max score if it is not None - max_score *= float(self.weight) + max_score *= float(weight) else: # Without a max_score, we cannot have a score! score = None diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index f92971796c..6164a4d635 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -57,7 +57,7 @@ class PeerGradingFields(object): scope=Scope.settings ) max_grade = Integer( - help="The maximum grade that a student can receive for this problem.", + help="The maximum grade that a student can receive for this problem.", default=MAX_SCORE, scope=Scope.settings, values={"min": 0} @@ -214,6 +214,11 @@ class PeerGradingModule(PeerGradingFields, XModule): def get_score(self): max_score = None score = None + weight = self.weight + + #The old default was None, so set to 1 if it is the old default weight + if weight is None: + weight = 1 score_dict = { 'score': score, 'total': max_score, @@ -238,11 +243,10 @@ class PeerGradingModule(PeerGradingFields, XModule): # Ensures that once a student receives a final score for peer grading, that it does not change. self.student_data_for_location = response - if self.weight is not None: - score = int(count_graded >= count_required and count_graded > 0) * float(self.weight) - total = self.max_grade * float(self.weight) - score_dict['score'] = score - score_dict['total'] = total + score = int(count_graded >= count_required and count_graded > 0) * float(weight) + total = self.max_grade * float(weight) + score_dict['score'] = score + score_dict['total'] = total return score_dict From b1653f056143216ec0026c2f6efcc30df2e2a680 Mon Sep 17 00:00:00 2001 From: Frances Botsford Date: Thu, 11 Jul 2013 11:19:39 -0400 Subject: [PATCH 008/112] layout cleanup on LMS PDF Textbook viewer --- common/static/css/pdfviewer.css | 3 +- lms/static/sass/course/_textbook.scss | 59 +++++++++++++++++++---- lms/templates/static_pdfbook.html | 67 ++++++++++++++------------- 3 files changed, 87 insertions(+), 42 deletions(-) diff --git a/common/static/css/pdfviewer.css b/common/static/css/pdfviewer.css index 656bc47c29..8b0253261b 100644 --- a/common/static/css/pdfviewer.css +++ b/common/static/css/pdfviewer.css @@ -100,7 +100,7 @@ select { .toolbar { /* position: absolute; */ left: 0; - right: 0; + right: 0; height: 32px; z-index: 9999; cursor: default; @@ -185,6 +185,7 @@ select { margin: 0; } +.splitToolbarButton > .toolbarButton, /*added */ .splitToolbarButton:hover > .toolbarButton, .splitToolbarButton:focus > .toolbarButton, .splitToolbarButton.toggled > .toolbarButton, diff --git a/lms/static/sass/course/_textbook.scss b/lms/static/sass/course/_textbook.scss index bc9da1f43f..08f1a853dc 100644 --- a/lms/static/sass/course/_textbook.scss +++ b/lms/static/sass/course/_textbook.scss @@ -1,13 +1,46 @@ div.book-wrapper { - display: table; - table-layout: fixed; - padding: 1em 8em; + max-width: 1150px; + margin: 0 auto; + width: 100%; + background-color: $white; + #toolbarViewer { + padding: 0 ($baseline/2); + + #toolbarViewerLeft { + display: inline-block; + } + + .outerCenter { + display: inline-block; + float: right !important; + right: auto; + + .innerCenter { + right: auto; + } + + .dropdownToolbarButton { + margin: 3px 2px 4px 0; + } + } + + + } #open_close_accordion { display: none; } + .pdfbook-wrap { + display: table; + width: 100%; + } + + .pdfbook-wrap-inner { + display: table-row; + } + section.book-sidebar { @extend .sidebar; @extend .tran; @@ -44,14 +77,17 @@ div.book-wrapper { li { background: none; border-bottom: 0; - padding-left: lh(); + padding-left: ($baseline/2); a { - padding: 0; @include clearfix; + padding: 0; + color: $link-color; + cursor: pointer; &:hover { background-color: transparent; + color: $link-hover; .page-number { opacity: 1.0; @@ -84,7 +120,7 @@ div.book-wrapper { > li { padding: 5px 6px; - margin: 0 16px 5px 25px; + margin: ($baseline/4) ($baseline/2); } } } @@ -158,18 +194,20 @@ div.book-wrapper { } section.page { - border: 1px solid $border-color; + border-left: 1px solid $border-color; background-color: #fff; position: relative; text-align: center; - padding: lh(); - margin-right:10%; border-radius: 0 3px 3px 0; img { max-width: 100%; } + #viewer { + padding: $baseline; + } + div { text-align: left; line-height: 1.6em; @@ -214,3 +252,6 @@ div.book-wrapper { } } } + + + diff --git a/lms/templates/static_pdfbook.html b/lms/templates/static_pdfbook.html index 565a59977a..a8608b2877 100644 --- a/lms/templates/static_pdfbook.html +++ b/lms/templates/static_pdfbook.html @@ -11,7 +11,7 @@ <%static:js group='courseware'/> - + <%block name="js_extra"> @@ -35,10 +35,10 @@ %if page is not None: options.pageNum = ${page}; %endif - + $('#outerContainer').PDFViewer(options); }); - + <%include file="/courseware/course_navigation.html" args="active_page='pdftextbook/{0}'.format(book_index)" /> @@ -91,40 +91,43 @@
%if 'chapters' in textbook: -
- +
%endif -
+
- -
-
-
- + +
+
+ + + From 08fe23ac5f0c872b1b5784cb81cdc6f2e1c98a61 Mon Sep 17 00:00:00 2001 From: Frances Botsford Date: Thu, 11 Jul 2013 11:26:59 -0400 Subject: [PATCH 009/112] adjusted the LMS PDF textbook zoom elements positioning --- lms/static/sass/course/_textbook.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/static/sass/course/_textbook.scss b/lms/static/sass/course/_textbook.scss index 08f1a853dc..72d73bdb78 100644 --- a/lms/static/sass/course/_textbook.scss +++ b/lms/static/sass/course/_textbook.scss @@ -13,8 +13,8 @@ div.book-wrapper { .outerCenter { display: inline-block; - float: right !important; - right: auto; + position: absolute; + right: ($baseline/2); .innerCenter { right: auto; From 84f531af54f91f7aa1ef7e28afc698db778c49c4 Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 12 Jul 2013 13:40:55 +0000 Subject: [PATCH 010/112] add export_all_courses management script to cms --- .../management/commands/export.py | 2 +- .../management/commands/export_all_courses.py | 49 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 cms/djangoapps/contentstore/management/commands/export_all_courses.py diff --git a/cms/djangoapps/contentstore/management/commands/export.py b/cms/djangoapps/contentstore/management/commands/export.py index eb7800d46c..0087fdddcb 100644 --- a/cms/djangoapps/contentstore/management/commands/export.py +++ b/cms/djangoapps/contentstore/management/commands/export.py @@ -14,7 +14,7 @@ unnamed_modules = 0 class Command(BaseCommand): - help = 'Import the specified data directory into the default ModuleStore' + help = 'Export the specified data directory into the default ModuleStore' def handle(self, *args, **options): if len(args) != 2: diff --git a/cms/djangoapps/contentstore/management/commands/export_all_courses.py b/cms/djangoapps/contentstore/management/commands/export_all_courses.py new file mode 100644 index 0000000000..978418ba7b --- /dev/null +++ b/cms/djangoapps/contentstore/management/commands/export_all_courses.py @@ -0,0 +1,49 @@ +### +### Script for exporting all courseware from Mongo to a directory +### +import os + +from django.core.management.base import BaseCommand, CommandError +from xmodule.modulestore.xml_exporter import export_to_xml +from xmodule.modulestore.django import modulestore +from xmodule.contentstore.django import contentstore +from xmodule.course_module import CourseDescriptor + + +unnamed_modules = 0 + + +class Command(BaseCommand): + help = 'Export all courses from mongo to the specified data directory' + + def handle(self, *args, **options): + if len(args) != 1: + raise CommandError("import requires one argument: ") + + output_path = args[0] + + cs = contentstore() + ms = modulestore('direct') + root_dir = output_path + courses = ms.get_courses() + + print "%d courses to export:" % len(courses) + cids = [x.id for x in courses] + print cids + + for course_id in cids: + + print "-"*77 + print "Exporting course id = {0} to {1}".format(course_id, output_path) + + if 1: + try: + location = CourseDescriptor.id_to_location(course_id) + course_dir = course_id.replace('/','...') + export_to_xml(ms, cs, location, root_dir, course_dir) + except Exception as err: + print "="*30 + "> Oops, failed to export %s" % course_id + print "Error:" + print err + + From 84c38b4e9a58e6ea2c317212a8fe96dc58195c32 Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 12 Jul 2013 09:43:52 -0400 Subject: [PATCH 011/112] fix two typos in export scripts --- cms/djangoapps/contentstore/management/commands/export.py | 2 +- .../contentstore/management/commands/export_all_courses.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/export.py b/cms/djangoapps/contentstore/management/commands/export.py index 0087fdddcb..52a5eb265b 100644 --- a/cms/djangoapps/contentstore/management/commands/export.py +++ b/cms/djangoapps/contentstore/management/commands/export.py @@ -18,7 +18,7 @@ class Command(BaseCommand): def handle(self, *args, **options): if len(args) != 2: - raise CommandError("import requires two arguments: ") + raise CommandError("export requires two arguments: ") course_id = args[0] output_path = args[1] diff --git a/cms/djangoapps/contentstore/management/commands/export_all_courses.py b/cms/djangoapps/contentstore/management/commands/export_all_courses.py index 978418ba7b..5d0adaa315 100644 --- a/cms/djangoapps/contentstore/management/commands/export_all_courses.py +++ b/cms/djangoapps/contentstore/management/commands/export_all_courses.py @@ -18,7 +18,7 @@ class Command(BaseCommand): def handle(self, *args, **options): if len(args) != 1: - raise CommandError("import requires one argument: ") + raise CommandError("export requires one argument: ") output_path = args[0] From 08b8438dda9a5e6ec2e8b4b3136a189160bf561d Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 12 Jul 2013 14:10:16 +0000 Subject: [PATCH 012/112] add extra modulestore() argument to export to make it export drafts also --- .../contentstore/management/commands/export_all_courses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/management/commands/export_all_courses.py b/cms/djangoapps/contentstore/management/commands/export_all_courses.py index 5d0adaa315..3c7c398e94 100644 --- a/cms/djangoapps/contentstore/management/commands/export_all_courses.py +++ b/cms/djangoapps/contentstore/management/commands/export_all_courses.py @@ -40,7 +40,7 @@ class Command(BaseCommand): try: location = CourseDescriptor.id_to_location(course_id) course_dir = course_id.replace('/','...') - export_to_xml(ms, cs, location, root_dir, course_dir) + export_to_xml(ms, cs, location, root_dir, course_dir, modulestore()) except Exception as err: print "="*30 + "> Oops, failed to export %s" % course_id print "Error:" From c53dd97f79827a3cfd818cf90f88f6bc94319cd4 Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 12 Jul 2013 14:11:20 +0000 Subject: [PATCH 013/112] add extra modulestore() argument to single export script --- cms/djangoapps/contentstore/management/commands/export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/management/commands/export.py b/cms/djangoapps/contentstore/management/commands/export.py index 52a5eb265b..90db8750d9 100644 --- a/cms/djangoapps/contentstore/management/commands/export.py +++ b/cms/djangoapps/contentstore/management/commands/export.py @@ -30,4 +30,4 @@ class Command(BaseCommand): root_dir = os.path.dirname(output_path) course_dir = os.path.splitext(os.path.basename(output_path))[0] - export_to_xml(modulestore('direct'), contentstore(), location, root_dir, course_dir) + export_to_xml(modulestore('direct'), contentstore(), location, root_dir, course_dir, modulestore()) From f27ca66f51e555779f1ebba6f5167671d55dd192 Mon Sep 17 00:00:00 2001 From: JonahStanley Date: Fri, 12 Jul 2013 10:11:39 -0400 Subject: [PATCH 014/112] Added documentation to new acceptance test features and techniques --- doc/testing.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/doc/testing.md b/doc/testing.md index b40ba30610..96f0dcfd98 100644 --- a/doc/testing.md +++ b/doc/testing.md @@ -175,6 +175,8 @@ Use `rake -T` to get a list of all available subsystems **Troubleshooting**: If you get an error message while running the `rake` task, try running `bundle install` to install the required ruby gems. +Unit tests can be run in parallel to each other and while acceptance tests are running + ### Running Acceptance Tests We use [Lettuce](http://lettuce.it/) for acceptance testing. @@ -203,6 +205,10 @@ To start the debugger on failure, add the `--pdb` option: To run tests faster by not collecting static files, you can use `rake fasttest_acceptance_lms` and `rake fasttest_acceptance_cms`. +Acceptance tests will run on a randomized port and can be run in the background of rake cms and lms or unit tests. +To specify the port, change the LETTUCE_SERVER_PORT constant in cms/envs/acceptance.py and lms/envs/acceptance.py +as well as the port listed in cms/djangoapps/contentstore/feature/upload.py + **Note**: The acceptance tests can *not* currently run in parallel. ## Viewing Test Coverage @@ -230,3 +236,30 @@ When testing problems that use a queue server on AWS (e.g. sandbox-xqueue.edx.or `django-admin.py runserver --settings=lms.envs.dev --pythonpath=. 0.0.0.0:8000` When you connect to the LMS, you need to use the public ip. Use `ifconfig` to figure out the number, and connect e.g. to `http://18.3.4.5:8000/` + + +## Acceptance Test Techniques + +1. Do not assert not if possible for css. Use world.is_css_present and is_css_not_present + Errors can arise if checks for the css are performed before the page finishes loading. + To get around this, there are functions that will wait a period of time for the css to appear + before returning and return immediately if they are there. There is a reverse to this function as well. + It will wait for the css to not appear and returns if it isn't there. + + All css functions can utilize this timeout to ensure that the page is fully loaded + +2. Dealing with alerts + Chrome can hang on javascripts alerts. If a javascript alert/prompt/confirmation is expected, use the step + 'I will confirm all alerts', 'I will cancel all alerts' or 'I will anser all prompts with "(.*)"' before the step + that causes the alert in order to properly deal with it. + +3. Dealing with stale element reference exceptions + These exceptions happen if any part of the page is refreshed in between finding an element and accessing the element. + When possible, use any of the css functions in common/djangoapps/terrain/ui_helpers.py as they will retry the action + in case of this exception. If the functionality is not there, wrap the function with world.retry_on_exception. This function takes in a function and will retry and return the result of the function if there was an exception + +4. Scenario Level Constants + If you want an object to be available for the entire scenario, it can be stored in world.scenario_dict. This object + is a dictionary that gets refreshed at the beginning on the scenario. Currently, the current logged in user and the current created course are stored under 'COURSE' and 'USER'. This will help prevent strings from being hard coded so the + acceptance tests can become more flexible. + From bdca0f1d0e5be6fe15a13d48e4b7e2bd9f55eb1c Mon Sep 17 00:00:00 2001 From: Xavier Antoviaque Date: Fri, 12 Jul 2013 11:15:00 -0300 Subject: [PATCH 015/112] vagrant: Always mount `node_modules` folder after Vagrant NFS mount --- scripts/vagrant-provisioning.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/scripts/vagrant-provisioning.sh b/scripts/vagrant-provisioning.sh index 153bc7a879..d4f627b2c4 100755 --- a/scripts/vagrant-provisioning.sh +++ b/scripts/vagrant-provisioning.sh @@ -59,7 +59,14 @@ chown vagrant.vagrant ~vagrant/.ssh/known_hosts # Node modules require a filesystem with symlinks (Windows support) mkdir -p /opt/edx/node_modules /opt/edx/edx-platform/node_modules -mount -o bind /opt/edx/node_modules /opt/edx/edx-platform/node_modules +([[ -f /etc/fstab ]] && grep '/opt/edx/node_modules' /etc/fstab) || { + echo '/opt/edx/node_modules /opt/edx/edx-platform/node_modules none bind,noauto 0 0' >> /etc/fstab + mount /opt/edx/node_modules +} +# Must be mounted *after* the NFS mount, made manually by Vagrant +([[ -f /etc/cron.d/nodemodules ]] && grep '/opt/edx/node_modules' /etc/cron.d/nodemodules) || { + echo '@reboot root until [ -n "`mount |grep "/opt/edx/edx-platform type"`" ]; do sleep 1; done; mount /opt/edx/node_modules' > /etc/cron.d/nodemodules +} # Force rechecking all prerequisites (could have been fetched outside of the VM) rm -rf /opt/edx/edx-platform/.prereqs_cache From e62cb45da4242578f4369afdf40b4f47c1474336 Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 8 Jul 2013 15:50:22 +0000 Subject: [PATCH 016/112] tracking in idashboard should only log json-serializable objects --- lms/djangoapps/instructor/views.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 9f9b7a2399..b046bdb762 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -374,7 +374,7 @@ def instructor_dashboard(request, course_id): msg += message if student is not None: progress_url = reverse('student_progress', kwargs={'course_id': course_id, 'student_id': student.id}) - track.views.server_track(request, "get-student-progress-page", {"student": student, "instructor": request.user, "course": course_id}, page="idashboard") + track.views.server_track(request, "get-student-progress-page", {"student": str(student), "instructor": str(request.user), "course": course_id}, page="idashboard") msg += " Progress page for username: {1} with email address: {2}.".format(progress_url, student.username, student.email) #---------------------------------------- @@ -472,7 +472,7 @@ def instructor_dashboard(request, course_id): msg += 'Added {0} to instructor group = {1}'.format(user, group.name) log.debug('staffgrp={0}'.format(group.name)) user.groups.add(group) - track.views.server_track(request, "add-instructor", {"instructor": user}, page="idashboard") + track.views.server_track(request, "add-instructor", {"instructor": str(user)}, page="idashboard") elif action == 'Remove course staff': uname = request.POST['staffuser'] @@ -491,7 +491,7 @@ def instructor_dashboard(request, course_id): msg += 'Removed {0} from instructor group = {1}'.format(user, group.name) log.debug('instructorgrp={0}'.format(group.name)) user.groups.remove(group) - track.views.server_track(request, "remove-instructor", {"instructor": user}, page="idashboard") + track.views.server_track(request, "remove-instructor", {"instructor": str(user)}, page="idashboard") #---------------------------------------- # DataDump @@ -658,7 +658,7 @@ def instructor_dashboard(request, course_id): problem = request.POST['Problem'] nmsg, plots = psychoanalyze.generate_plots_for_problem(problem) msg += nmsg - track.views.server_track(request, "psychometrics-histogram-generation", {"problem": problem}, page="idashboard") + track.views.server_track(request, "psychometrics-histogram-generation", {"problem": str(problem)}, page="idashboard") if idash_mode == 'Psychometrics': problems = psychoanalyze.problems_with_psychometric_data(course_id) @@ -911,7 +911,7 @@ def _add_or_remove_user_group(request, username_or_email, group, group_title, ev else: user.groups.remove(group) event = "add" if do_add else "remove" - track.views.server_track(request, "add-or-remove-user-group", {"event_name": event_name, "user": user, "event": event}, page="idashboard") + track.views.server_track(request, "add-or-remove-user-group", {"event_name": event_name, "user": str(user), "event": event}, page="idashboard") return msg From a3e4f21a9c891735765f96e61fd302e8bb774c17 Mon Sep 17 00:00:00 2001 From: ichuang Date: Tue, 9 Jul 2013 14:24:23 -0400 Subject: [PATCH 017/112] use unicode instead of str for casting track.views args in idashboard --- lms/djangoapps/instructor/views.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index b046bdb762..d133bdaf38 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -374,7 +374,7 @@ def instructor_dashboard(request, course_id): msg += message if student is not None: progress_url = reverse('student_progress', kwargs={'course_id': course_id, 'student_id': student.id}) - track.views.server_track(request, "get-student-progress-page", {"student": str(student), "instructor": str(request.user), "course": course_id}, page="idashboard") + track.views.server_track(request, "get-student-progress-page", {"student": unicode(student), "instructor": unicode(request.user), "course": course_id}, page="idashboard") msg += " Progress page for username: {1} with email address: {2}.".format(progress_url, student.username, student.email) #---------------------------------------- @@ -472,7 +472,7 @@ def instructor_dashboard(request, course_id): msg += 'Added {0} to instructor group = {1}'.format(user, group.name) log.debug('staffgrp={0}'.format(group.name)) user.groups.add(group) - track.views.server_track(request, "add-instructor", {"instructor": str(user)}, page="idashboard") + track.views.server_track(request, "add-instructor", {"instructor": unicode(user)}, page="idashboard") elif action == 'Remove course staff': uname = request.POST['staffuser'] @@ -491,7 +491,7 @@ def instructor_dashboard(request, course_id): msg += 'Removed {0} from instructor group = {1}'.format(user, group.name) log.debug('instructorgrp={0}'.format(group.name)) user.groups.remove(group) - track.views.server_track(request, "remove-instructor", {"instructor": str(user)}, page="idashboard") + track.views.server_track(request, "remove-instructor", {"instructor": unicode(user)}, page="idashboard") #---------------------------------------- # DataDump @@ -658,7 +658,7 @@ def instructor_dashboard(request, course_id): problem = request.POST['Problem'] nmsg, plots = psychoanalyze.generate_plots_for_problem(problem) msg += nmsg - track.views.server_track(request, "psychometrics-histogram-generation", {"problem": str(problem)}, page="idashboard") + track.views.server_track(request, "psychometrics-histogram-generation", {"problem": unicode(problem)}, page="idashboard") if idash_mode == 'Psychometrics': problems = psychoanalyze.problems_with_psychometric_data(course_id) @@ -911,7 +911,7 @@ def _add_or_remove_user_group(request, username_or_email, group, group_title, ev else: user.groups.remove(group) event = "add" if do_add else "remove" - track.views.server_track(request, "add-or-remove-user-group", {"event_name": event_name, "user": str(user), "event": event}, page="idashboard") + track.views.server_track(request, "add-or-remove-user-group", {"event_name": event_name, "user": unicode(user), "event": event}, page="idashboard") return msg From 50bea28d750a8d5bfbe9ff31b5bdef18c2957bdd Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 12 Jul 2013 11:13:59 -0400 Subject: [PATCH 018/112] Add a 'string literal' serialization method and use it in the lambda function mapping table when the value to xml serialize is of basestring type. Also a couple of drive-by pep8 fixes. --- common/lib/xmodule/xmodule/xml_module.py | 54 ++++++++++++++---------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index c1340a9fc0..edd10288de 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -81,13 +81,23 @@ class AttrMap(_AttrMapBase): def serialize_field(value): """ - Return a string version of the value (where value is the JSON-formatted, internally stored value). + Return a string version of the value (where value is the JSON-formatted, internally stored value). - By default, this is the result of calling json.dumps on the input value. - """ + By default, this is the result of calling json.dumps on the input value. + """ return json.dumps(value, cls=EdxJSONEncoder) +def serialize_string_literal(value): + """ + Assert that the value is a base string and - if it is - simply return it + """ + if not isinstance(value, basestring): + raise Exception('Value {0} is not of type basestring!'.format(value)) + + return value + + def deserialize_field(field, value): """ Deserialize the string version to the value stored internally. @@ -126,7 +136,7 @@ class XmlDescriptor(XModuleDescriptor): """ xml_attributes = Dict(help="Map of unhandled xml attributes, used only for storage between import and export", - default={}, scope=Scope.settings) + default={}, scope=Scope.settings) # Extension to append to filename paths filename_extension = 'xml' @@ -141,23 +151,23 @@ class XmlDescriptor(XModuleDescriptor): # understand? And if we do, is this the place? # Related: What's the right behavior for clean_metadata? metadata_attributes = ('format', 'graceperiod', 'showanswer', 'rerandomize', - 'start', 'due', 'graded', 'display_name', 'url_name', 'hide_from_toc', - 'ispublic', # if True, then course is listed for all users; see - 'xqa_key', # for xqaa server access - 'giturl', # url of git server for origin of file - # information about testcenter exams is a dict (of dicts), not a string, - # so it cannot be easily exportable as a course element's attribute. - 'testcenter_info', - # VS[compat] Remove once unused. - 'name', 'slug') + 'start', 'due', 'graded', 'display_name', 'url_name', 'hide_from_toc', + 'ispublic', # if True, then course is listed for all users; see + 'xqa_key', # for xqaa server access + 'giturl', # url of git server for origin of file + # information about testcenter exams is a dict (of dicts), not a string, + # so it cannot be easily exportable as a course element's attribute. + 'testcenter_info', + # VS[compat] Remove once unused. + 'name', 'slug') metadata_to_strip = ('data_dir', - 'tabs', 'grading_policy', 'published_by', 'published_date', - 'discussion_blackouts', 'testcenter_info', - # VS[compat] -- remove the below attrs once everything is in the CMS - 'course', 'org', 'url_name', 'filename', - # Used for storing xml attributes between import and export, for roundtrips - 'xml_attributes') + 'tabs', 'grading_policy', 'published_by', 'published_date', + 'discussion_blackouts', 'testcenter_info', + # VS[compat] -- remove the below attrs once everything is in the CMS + 'course', 'org', 'url_name', 'filename', + # Used for storing xml attributes between import and export, for roundtrips + 'xml_attributes') metadata_to_export_to_policy = ('discussion_topics') @@ -166,7 +176,7 @@ class XmlDescriptor(XModuleDescriptor): for field in set(cls.fields + cls.lms.fields): if field.name == attr: from_xml = lambda val: deserialize_field(field, val) - to_xml = lambda val : serialize_field(val) + to_xml = lambda val: serialize_string_literal(val) if isinstance(val, basestring) else serialize_field(val) return AttrMap(from_xml, to_xml) return AttrMap() @@ -254,7 +264,7 @@ class XmlDescriptor(XModuleDescriptor): definition, children = cls.definition_from_xml(definition_xml, system) if definition_metadata: definition['definition_metadata'] = definition_metadata - definition['filename'] = [ filepath, filename ] + definition['filename'] = [filepath, filename] return definition, children @@ -280,7 +290,6 @@ class XmlDescriptor(XModuleDescriptor): metadata[attr] = attr_map.from_xml(val) return metadata - @classmethod def apply_policy(cls, metadata, policy): """ @@ -374,7 +383,6 @@ class XmlDescriptor(XModuleDescriptor): """ return True - def export_to_xml(self, resource_fs): """ Returns an xml string representing this module, and all modules From 5aae3c69715bb84294354299f26fbcf07388c0d6 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 12 Jul 2013 11:14:35 -0400 Subject: [PATCH 019/112] add new tests for the string literal serialization --- .../xmodule/xmodule/tests/test_xml_module.py | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index 6581ce58f6..a853e6ab3f 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -4,7 +4,7 @@ from xmodule.x_module import XModuleFields from xblock.core import Scope, String, Dict, Boolean, Integer, Float, Any, List from xmodule.fields import Date, Timedelta -from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field +from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field, serialize_string_literal import unittest from .import get_test_system from nose.tools import assert_equals @@ -186,12 +186,27 @@ class TestSerialize(unittest.TestCase): assert_equals('"false"', serialize_field('false')) assert_equals('"fAlse"', serialize_field('fAlse')) assert_equals('"hat box"', serialize_field('hat box')) - assert_equals('{"bar": "hat", "frog": "green"}', serialize_field({'bar': 'hat', 'frog' : 'green'})) + assert_equals('{"bar": "hat", "frog": "green"}', serialize_field({'bar': 'hat', 'frog': 'green'})) assert_equals('[3.5, 5.6]', serialize_field([3.5, 5.6])) assert_equals('["foo", "bar"]', serialize_field(['foo', 'bar'])) assert_equals('"2012-12-31T23:59:59Z"', serialize_field("2012-12-31T23:59:59Z")) assert_equals('"1 day 12 hours 59 minutes 59 seconds"', - serialize_field("1 day 12 hours 59 minutes 59 seconds")) + serialize_field("1 day 12 hours 59 minutes 59 seconds")) + + def test_serialize_string_literal(self): + assert_equals('2', serialize_string_literal('2')) + assert_equals('2.589', serialize_string_literal('2.589')) + assert_equals('false', serialize_string_literal('false')) + assert_equals('fAlse', serialize_string_literal('fAlse')) + assert_equals('hat box', serialize_string_literal('hat box')) + assert_equals('2012-12-31T23:59:59Z', serialize_string_literal("2012-12-31T23:59:59Z")) + assert_equals('1 day 12 hours 59 minutes 59 seconds', + serialize_string_literal("1 day 12 hours 59 minutes 59 seconds")) + + try: + self.assertRaises(serialize_string_literal(2.31)) + except Exception: + pass class TestDeserialize(unittest.TestCase): From ab4012cc594341ac78663bd89f90621cb8c02bd3 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 12 Jul 2013 11:33:46 -0400 Subject: [PATCH 020/112] fix up some of the test courses which had hardcoded that quot; escaping --- common/test/data/simple/course.xml | 8 ++++---- .../test/data/test_exam_registration/course/2012_Fall.xml | 4 ++-- common/test/data/test_start_date/course/2012_Fall.xml | 4 ++-- common/test/data/toy/chapter/secret/magic.xml | 2 +- common/test/data/toy/course/2012_Fall.xml | 8 ++++---- common/test/data/two_toys/video/Video_Resources.xml | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/common/test/data/simple/course.xml b/common/test/data/simple/course.xml index c9bb5ec8b2..529528ca0a 100644 --- a/common/test/data/simple/course.xml +++ b/common/test/data/simple/course.xml @@ -1,13 +1,13 @@ -