From c2e5c607136e93a8c63d1aa073fe0ef1756a24f3 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 13 Jun 2013 15:34:00 -0400 Subject: [PATCH 01/73] 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 02/73] 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 03/73] 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 04/73] 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 05/73] 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 06/73] 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 07/73] 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 08/73] 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 09/73] 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 e62cb45da4242578f4369afdf40b4f47c1474336 Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 8 Jul 2013 15:50:22 +0000 Subject: [PATCH 10/73] 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 11/73] 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 c9943306efdbd799508d7beb199bbcb3eafc2425 Mon Sep 17 00:00:00 2001 From: Jason Bau Date: Fri, 12 Jul 2013 22:57:13 -0700 Subject: [PATCH 12/73] put block around main_vendor js in main_django.html should be a no-op for edx-east, but allow edx-west to remove vendor.js with a blank {% block %} in our password_reset_confirm.html to fix a bug --- lms/templates/main_django.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lms/templates/main_django.html b/lms/templates/main_django.html index f5ee06d280..da3af1935c 100644 --- a/lms/templates/main_django.html +++ b/lms/templates/main_django.html @@ -7,7 +7,9 @@ {% compressed_css 'application' %} + {% block main_vendor_js %} {% compressed_js 'main_vendor' %} + {% endblock %} {% block headextra %}{% endblock %} {% render_block "css" %} From d83d49cac40fd1720d7b8c07146002c1c94de7e7 Mon Sep 17 00:00:00 2001 From: Lyla Fischer Date: Mon, 15 Jul 2013 11:26:03 -0400 Subject: [PATCH 13/73] drag_and_drop template added --- .../templates/problem/drag_and_drop.yaml | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 common/lib/xmodule/xmodule/templates/problem/drag_and_drop.yaml diff --git a/common/lib/xmodule/xmodule/templates/problem/drag_and_drop.yaml b/common/lib/xmodule/xmodule/templates/problem/drag_and_drop.yaml new file mode 100644 index 0000000000..3705004316 --- /dev/null +++ b/common/lib/xmodule/xmodule/templates/problem/drag_and_drop.yaml @@ -0,0 +1,44 @@ +--- +metadata: + display_name: Drag and Drop + rerandomize: never + showanswer: finished +data: | + + Here's an example of a "Drag and Drop" question set. Click and drag each word in the scrollbar below, up to the numbered bucket which matches the number of letters in the word. + + + + + + + + + + + + + + + + correct_answer = { + '1': [[70, 150], 121], + '6': [[190, 150], 121], + '8': [[190, 150], 121], + '2': [[310, 150], 121], + '9': [[310, 150], 121], + '11': [[310, 150], 121], + '4': [[420, 150], 121], + '7': [[420, 150], 121], + '3': [[550, 150], 121], + '5': [[550, 150], 121], + '10': [[550, 150], 121]} + if draganddrop.grade(submission[0], correct_answer): + correct = ['correct'] + else: + correct = ['incorrect'] + + + + +children: [] From 2262ffe4e58cbd07845eb6aae98cb1db00e38fbe Mon Sep 17 00:00:00 2001 From: Lyla Fischer Date: Mon, 15 Jul 2013 11:58:37 -0400 Subject: [PATCH 14/73] updated imageresponse example to a more universal example (baby animals) --- .../templates/problem/imageresponse.yaml | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml b/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml index ab1f22e3b2..d9acb9359c 100644 --- a/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml @@ -5,22 +5,21 @@ metadata: showanswer: finished data: | -

- An image mapped input problem presents an image for the student. Input is - given by the location of mouse clicks on the image. Correctness of input can be evaluated based on expected dimensions of a rectangle. -

- -

Which object in this image is required by the fire code?

- - - - -
-

Explanation

-

The fire code requires that all exits be clearly marked, so the red exit sign is the correct answer.

-
- +

+ An image mapped input problem presents an image for the student. + Input is given by the location of mouse clicks on the image. + Correctness of input can be evaluated based on expected dimensions of a rectangle. +

+

Which animal shown below is a kitten?

+ + + + +
+

Explanation

+

The animal on the right is a kitten. The animal on the left is a puppy, not a kitten.

+
+ - children: [] From 7c5943a87c0a28aced02e3210b0078e5958c79c4 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 21 Jun 2013 13:05:48 -0400 Subject: [PATCH 15/73] Tweak behavior of submit buttons on register and login pages The buttons are not re-enabled if the registration/login submission succeeds (and, therefore, the user is about to be directed to another page). Also, any error message that is present does not disappear immediately before the page redirects. --- lms/templates/login.html | 4 ++-- lms/templates/register.html | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lms/templates/login.html b/lms/templates/login.html index 671ce5a3e7..5ab63a86c2 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -38,13 +38,12 @@ toggleSubmitButton(false); }); - $('#login-form').on('ajax:complete', function() { + $('#login-form').on('ajax:error', function() { toggleSubmitButton(true); }); $('#login-form').on('ajax:success', function(event, json, xhr) { if(json.success) { - $('.message.submission-error').removeClass('is-shown'); var u=decodeURI(window.location.search); next=u.split("next=")[1]; if (next) { @@ -53,6 +52,7 @@ location.href="${reverse('dashboard')}"; } } else { + toggleSubmitButton(true); $('.message.submission-error').addClass('is-shown').focus(); $('.message.submission-error .message-copy').html(json.value); } diff --git a/lms/templates/register.html b/lms/templates/register.html index 1c2490c0a0..57a9ffa843 100644 --- a/lms/templates/register.html +++ b/lms/templates/register.html @@ -45,15 +45,15 @@ toggleSubmitButton(false); }); - $('#register-form').on('ajax:complete', function() { + $('#register-form').on('ajax:error', function() { toggleSubmitButton(true); }); $('#register-form').on('ajax:success', function(event, json, xhr) { if(json.success) { - $('.message.submission-error').removeClass('is-shown'); location.href="${reverse('dashboard')}"; } else { + toggleSubmitButton(true); $('.status.message.submission-error').addClass('is-shown').focus(); $('.status.message.submission-error .message-copy').html(json.value).stop().css("display", "block"); $(".field-error").removeClass('field-error'); From c770458e6a7171e265d05f617c3abceeedc8c518 Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Mon, 15 Jul 2013 13:31:56 -0400 Subject: [PATCH 16/73] Reinstate problem_check event as Problem Checked event --- common/lib/xmodule/xmodule/js/src/capa/display.coffee | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index e29276936b..fbbba36134 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -162,8 +162,9 @@ class @Problem # maybe preferable to consolidate all dispatches to use FormData ### check_fd: => - # Calling check from check_fd will result in firing the 'problem_check' event twice, since it is also called in the check function. - #Logger.log 'problem_check', @answers + # Calling check from check_fd will result in firing the 'problem_check' event twice, + # since it is also called in the check function. + # Logger.log 'problem_check', @answers # If there are no file inputs in the problem, we can fall back on @check if $('input:file').length == 0 @@ -239,6 +240,11 @@ class @Problem check: => @check_waitfor() Logger.log 'problem_check', @answers + + # Segment.io + analytics.track "Problem Checked", + answers: @answers + $.postWithPrefix "#{@url}/problem_check", @answers, (response) => switch response.success when 'incorrect', 'correct' From 27b7e35bd36500ec36327ca91ee194645f6b94f5 Mon Sep 17 00:00:00 2001 From: Frances Botsford Date: Mon, 15 Jul 2013 13:54:04 -0400 Subject: [PATCH 17/73] i18n the PDF prev/next button text --- lms/templates/static_pdfbook.html | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/templates/static_pdfbook.html b/lms/templates/static_pdfbook.html index a8608b2877..754e519113 100644 --- a/lms/templates/static_pdfbook.html +++ b/lms/templates/static_pdfbook.html @@ -1,3 +1,4 @@ +<%! from django.utils.translation import ugettext as _ %> <%inherit file="main.html" /> <%namespace name='static' file='static_content.html'/> <%block name="title"> @@ -116,10 +117,10 @@ From 92ae50d415ad19c6742d1797c64dc7618a53b5fd Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Mon, 15 Jul 2013 15:18:01 -0400 Subject: [PATCH 18/73] Remove unused code and include problem ID of data we send --- common/lib/xmodule/xmodule/js/src/capa/display.coffee | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index fbbba36134..b7dbf6864d 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -22,7 +22,6 @@ class @Problem @$('section.action input:button').click @refreshAnswers @$('section.action input.check').click @check_fd - #@$('section.action input.check').click @check @$('section.action input.reset').click @reset @$('section.action button.show').click @show @$('section.action input.save').click @save @@ -162,10 +161,6 @@ class @Problem # maybe preferable to consolidate all dispatches to use FormData ### check_fd: => - # Calling check from check_fd will result in firing the 'problem_check' event twice, - # since it is also called in the check function. - # Logger.log 'problem_check', @answers - # If there are no file inputs in the problem, we can fall back on @check if $('input:file').length == 0 @check() @@ -243,6 +238,7 @@ class @Problem # Segment.io analytics.track "Problem Checked", + problem_id: @id answers: @answers $.postWithPrefix "#{@url}/problem_check", @answers, (response) => From 22302e3a46d09672245a12d2115aa8bd9e011482 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 15 Jul 2013 16:47:32 -0400 Subject: [PATCH 19/73] Make jasmine tests quieter --- .gitignore | 1 + rakelib/helpers.rb | 15 +++++++++++---- rakelib/jasmine.rake | 13 +++++++++---- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index b1a36e5f2e..4fd90cfe03 100644 --- a/.gitignore +++ b/.gitignore @@ -45,3 +45,4 @@ node_modules autodeploy.properties .ws_migrations_complete .vagrant/ +logs diff --git a/rakelib/helpers.rb b/rakelib/helpers.rb index 3373214a19..925b7e1b28 100644 --- a/rakelib/helpers.rb +++ b/rakelib/helpers.rb @@ -52,8 +52,14 @@ end # Runs Process.spawn, and kills the process at the end of the rake process # Expects the same arguments as Process.spawn -def background_process(*command) - pid = Process.spawn({}, *command, {:pgroup => true}) +def background_process(command, logfile=nil) + spawn_opts = {:pgroup => true} + if !logfile.nil? + puts "Running '#{command.join(' ')}', redirecting output to #{logfile}".red + spawn_opts[[:err, :out]] = [logfile, 'a'] + end + pid = Process.spawn({}, *command, spawn_opts) + command = [*command] at_exit do puts "Ending process and children" @@ -88,9 +94,10 @@ end # Runs a command as a background process, as long as no other processes # tagged with the same tag are running -def singleton_process(*command) +def singleton_process(command, logfile=nil) + command = [*command] if Sys::ProcTable.ps.select {|proc| proc.cmdline.include?(command.join(' '))}.empty? - background_process(*command) + background_process(command, logfile) else puts "Process '#{command.join(' ')} already running, skipping".blue end diff --git a/rakelib/jasmine.rake b/rakelib/jasmine.rake index ff72161937..5a0c4acedc 100644 --- a/rakelib/jasmine.rake +++ b/rakelib/jasmine.rake @@ -8,6 +8,11 @@ PREFERRED_METHOD = PHANTOMJS_PATH.nil? ? 'browser' : 'phantomjs' if PHANTOMJS_PATH.nil? puts("phantomjs not found on path. Set $PHANTOMJS_PATH. Using browser for jasmine tests".blue) end +LOGDIR = 'logs/jasmine' + +CLOBBER.include(LOGDIR) + +directory LOGDIR def django_for_jasmine(system, django_reload) if !django_reload @@ -17,7 +22,7 @@ def django_for_jasmine(system, django_reload) port = 10000 + rand(40000) jasmine_url = "http://localhost:#{port}/_jasmine/" - background_process(*django_admin(system, 'jasmine', 'runserver', '-v', '0', port.to_s, reload_arg).split(' ')) + background_process(django_admin(system, 'jasmine', 'runserver', '-v', '0', port.to_s, reload_arg).split(' '), "#{LOGDIR}/django.log") up = false start_time = Time.now @@ -80,7 +85,7 @@ end namespace :jasmine do namespace system do desc "Open jasmine tests for #{system} in your default browser" - task :browser => [:clean_reports_dir] do + task :browser => [:clean_reports_dir, LOGDIR] do Rake::Task[:assets].invoke(system, 'jasmine') django_for_jasmine(system, true) do |jasmine_url| jasmine_browser(jasmine_url) @@ -88,7 +93,7 @@ end end desc "Open jasmine tests for #{system} in your default browser, and dynamically recompile coffeescript" - task :'browser:watch' => [:clean_reports_dir, :'assets:coffee:_watch'] do + task :'browser:watch' => [:clean_reports_dir, :'assets:coffee:_watch', LOGDIR] do django_for_jasmine(system, true) do |jasmine_url| jasmine_browser(jasmine_url, jitter=0, wait=0) end @@ -97,7 +102,7 @@ end end desc "Use phantomjs to run jasmine tests for #{system} from the console" - task :phantomjs => [:clean_reports_dir] do + task :phantomjs => [:clean_reports_dir, LOGDIR] do Rake::Task[:assets].invoke(system, 'jasmine') phantomjs = ENV['PHANTOMJS_PATH'] || 'phantomjs' django_for_jasmine(system, false) do |jasmine_url| From ef8618f7adb0f8e8764703d3b3b4fda799f93af3 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Mon, 24 Jun 2013 12:52:50 -0400 Subject: [PATCH 20/73] Make DraftModuleStore mongo only DraftModuleStore was originally designed as a mixin, but never used that way, and with the upcoming changes to use the versioned module store, never will be. This changes removes a circular dependency between mongo.py and draft.py. --- common/lib/xmodule/xmodule/modulestore/tests/django_utils.py | 2 +- lms/djangoapps/courseware/tests/tests.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 6c5c1f66ca..c32d0bca4c 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -48,7 +48,7 @@ def draft_mongo_store_config(data_dir): return { 'default': { - 'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore', + 'ENGINE': 'xmodule.modulestore.mongo.draft.DraftModuleStore', 'OPTIONS': modulestore_options }, 'direct': { diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 17cc848ad3..bbdf97ca9b 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -8,7 +8,6 @@ from django.core.urlresolvers import reverse from django.test.utils import override_settings import xmodule.modulestore.django - from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore from xmodule.modulestore import Location @@ -134,7 +133,7 @@ class TestCoursesLoadTestCase_XmlModulestore(PageLoaderTestCase): def setUp(self): super(TestCoursesLoadTestCase_XmlModulestore, self).setUp() self.setup_user() - xmodule.modulestore.django._MODULESTORES = {} + xmodule.modulestore.django._MODULESTORES.clear() def test_toy_course_loads(self): module_class = 'xmodule.hidden_module.HiddenDescriptor' @@ -155,7 +154,7 @@ class TestCoursesLoadTestCase_MongoModulestore(PageLoaderTestCase): def setUp(self): super(TestCoursesLoadTestCase_MongoModulestore, self).setUp() self.setup_user() - xmodule.modulestore.django._MODULESTORES = {} + xmodule.modulestore.django._MODULESTORES.clear() modulestore().collection.drop() def test_toy_course_loads(self): From 35f66f84695baa88e7acd26c359523ecd278e608 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Tue, 16 Jul 2013 09:20:35 -0400 Subject: [PATCH 21/73] Update MathJax to the latest stable version (2.2), to fix IE10 bug LMS-194 --- common/templates/mathjax_include.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/templates/mathjax_include.html b/common/templates/mathjax_include.html index 803f2145a4..0ddbd68eee 100644 --- a/common/templates/mathjax_include.html +++ b/common/templates/mathjax_include.html @@ -33,4 +33,4 @@ - + From 3a095acced7fcc5d040681d4fbd923c9d25ec52c Mon Sep 17 00:00:00 2001 From: James Tauber Date: Tue, 16 Jul 2013 13:38:34 -0400 Subject: [PATCH 22/73] changed transifex project to edx-platform --- .tx/config | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.tx/config b/.tx/config index 540c4732af..9288418924 100644 --- a/.tx/config +++ b/.tx/config @@ -1,25 +1,25 @@ [main] host = https://www.transifex.com -[edx-studio.django-partial] +[edx-platform.django-partial] file_filter = conf/locale//LC_MESSAGES/django-partial.po source_file = conf/locale/en/LC_MESSAGES/django-partial.po source_lang = en type = PO -[edx-studio.djangojs] +[edx-platform.djangojs] file_filter = conf/locale//LC_MESSAGES/djangojs.po source_file = conf/locale/en/LC_MESSAGES/djangojs.po source_lang = en type = PO -[edx-studio.mako] +[edx-platform.mako] file_filter = conf/locale//LC_MESSAGES/mako.po source_file = conf/locale/en/LC_MESSAGES/mako.po source_lang = en type = PO -[edx-studio.messages] +[edx-platform.messages] file_filter = conf/locale//LC_MESSAGES/messages.po source_file = conf/locale/en/LC_MESSAGES/messages.po source_lang = en From 8c904f31a90af89dea026489d8e316e93cb85c20 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Tue, 16 Jul 2013 14:22:42 -0400 Subject: [PATCH 23/73] Move defaults from yaml templates to field definitions. This standardizes the XModule field default values to be the same as the values that are presented by studio when a component is added to a course. --- .../features/discussion-editor.py | 6 +- .../contentstore/features/html-editor.py | 2 +- .../contentstore/features/problem-editor.py | 14 +- .../contentstore/features/video-editor.py | 2 +- .../contentstore/tests/test_contentstore.py | 24 +- .../tests/test_course_settings.py | 2 - common/djangoapps/xmodule_modifiers.py | 2 +- .../lib/xmodule/xmodule/annotatable_module.py | 27 +- common/lib/xmodule/xmodule/capa_module.py | 27 +- .../xmodule/combined_open_ended_module.py | 54 +++- common/lib/xmodule/xmodule/course_module.py | 254 ++++++++++++++---- .../lib/xmodule/xmodule/discussion_module.py | 22 +- common/lib/xmodule/xmodule/html_module.py | 81 +++++- .../xmodule/xmodule/peer_grading_module.py | 9 + common/lib/xmodule/xmodule/raw_module.py | 2 +- .../xmodule/templates/about/empty.yaml | 6 +- .../xmodule/templates/about/overview.yaml | 79 +++--- .../templates/annotatable/default.yaml | 21 +- .../templates/combinedopenended/default.yaml | 38 +-- .../xmodule/templates/course/empty.yaml | 125 +-------- .../xmodule/templates/courseinfo/empty.yaml | 6 +- .../xmodule/templates/default/empty.yaml | 6 +- .../xmodule/templates/discussion/default.yaml | 10 +- .../xmodule/templates/html/announcement.yaml | 3 +- .../xmodule/xmodule/templates/html/empty.yaml | 8 +- .../xmodule/templates/html/latex_html.yaml | 14 +- .../templates/peer_grading/default.yaml | 10 +- .../templates/problem/blank_common.yaml | 5 + .../templates/problem/circuitschematic.yaml | 111 ++++---- .../xmodule/templates/problem/empty.yaml | 12 +- .../xmodule/templates/statictab/empty.yaml | 6 +- .../xmodule/templates/video/default.yaml | 6 +- .../xmodule/templates/videoalpha/default.yaml | 12 +- .../xmodule/templates/word_cloud/default.yaml | 6 +- common/lib/xmodule/xmodule/video_module.py | 15 ++ .../lib/xmodule/xmodule/videoalpha_module.py | 16 +- .../lib/xmodule/xmodule/word_cloud_module.py | 10 +- common/lib/xmodule/xmodule/x_module.py | 126 ++++++--- lms/xmodule_namespace.py | 8 +- 39 files changed, 699 insertions(+), 488 deletions(-) create mode 100644 common/lib/xmodule/xmodule/templates/problem/blank_common.yaml diff --git a/cms/djangoapps/contentstore/features/discussion-editor.py b/cms/djangoapps/contentstore/features/discussion-editor.py index ae3da3c458..a4a4b71668 100644 --- a/cms/djangoapps/contentstore/features/discussion-editor.py +++ b/cms/djangoapps/contentstore/features/discussion-editor.py @@ -17,9 +17,9 @@ def i_created_discussion_tag(step): def i_see_only_the_settings_and_values(step): world.verify_all_setting_entries( [ - ['Category', "Week 1", True], - ['Display Name', "Discussion Tag", True], - ['Subcategory', "Topic-Level Student-Visible Label", True] + ['Category', "Week 1", False], + ['Display Name', "Discussion Tag", False], + ['Subcategory', "Topic-Level Student-Visible Label", False] ]) diff --git a/cms/djangoapps/contentstore/features/html-editor.py b/cms/djangoapps/contentstore/features/html-editor.py index 054c0ea642..53462ba094 100644 --- a/cms/djangoapps/contentstore/features/html-editor.py +++ b/cms/djangoapps/contentstore/features/html-editor.py @@ -14,4 +14,4 @@ def i_created_blank_html_page(step): @step('I see only the HTML display name setting$') def i_see_only_the_html_display_name(step): - world.verify_all_setting_entries([['Display Name', "Blank HTML Page", True]]) + world.verify_all_setting_entries([['Display Name', "Blank HTML Page", False]]) diff --git a/cms/djangoapps/contentstore/features/problem-editor.py b/cms/djangoapps/contentstore/features/problem-editor.py index 5d12b23d90..15f5da95e9 100644 --- a/cms/djangoapps/contentstore/features/problem-editor.py +++ b/cms/djangoapps/contentstore/features/problem-editor.py @@ -18,8 +18,9 @@ def i_created_blank_common_problem(step): world.create_component_instance( step, '.large-problem-icon', - 'i4x://edx/templates/problem/Blank_Common_Problem', - '.xmodule_CapaModule' + 'problem', + '.xmodule_CapaModule', + 'blank_common.yaml' ) @@ -32,11 +33,12 @@ def i_edit_and_select_settings(step): def i_see_five_settings_with_values(step): world.verify_all_setting_entries( [ - [DISPLAY_NAME, "Blank Common Problem", True], + [DISPLAY_NAME, "New problem", True], [MAXIMUM_ATTEMPTS, "", False], [PROBLEM_WEIGHT, "", False], - [RANDOMIZATION, "Never", True], - [SHOW_ANSWER, "Finished", True] + # Not sure why these are True other than via inspection + [RANDOMIZATION, "Always", True], + [SHOW_ANSWER, "Closed", True] ]) @@ -203,7 +205,7 @@ def verify_modified_display_name_with_special_chars(): def verify_unset_display_name(): - world.verify_setting_entry(world.get_setting_entry(DISPLAY_NAME), DISPLAY_NAME, '', False) + world.verify_setting_entry(world.get_setting_entry(DISPLAY_NAME), DISPLAY_NAME, 'Blank Advanced Problem', False) def set_weight(weight): diff --git a/cms/djangoapps/contentstore/features/video-editor.py b/cms/djangoapps/contentstore/features/video-editor.py index a6865fdd6d..e0f76b30ad 100644 --- a/cms/djangoapps/contentstore/features/video-editor.py +++ b/cms/djangoapps/contentstore/features/video-editor.py @@ -7,7 +7,7 @@ from lettuce import world, step @step('I see the correct settings and default values$') def i_see_the_correct_settings_and_values(step): world.verify_all_setting_entries([['Default Speed', 'OEoXaMPEzfM', False], - ['Display Name', 'default', True], + ['Display Name', 'Video Title', False], ['Download Track', '', False], ['Download Video', '', False], ['Show Captions', 'True', False], diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index be122fa1a4..6099b60eb1 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -135,7 +135,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.check_components_on_page(ADVANCED_COMPONENT_TYPES, ['Video Alpha', 'Word cloud', 'Annotation', - 'Open Ended Response', + 'Open Ended Grading', 'Peer Grading Interface']) def test_advanced_components_require_two_clicks(self): @@ -1271,6 +1271,28 @@ class ContentStoreTest(ModuleStoreTestCase): self.assertEqual(timedelta(1), new_module.lms.graceperiod) + def test_default_metadata_inheritance(self): + course = CourseFactory.create() + vertical = ItemFactory.create(parent_location=course.location) + course.children.append(vertical) + # in memory + self.assertIsNotNone(course.start) + self.assertEqual(course.start, vertical.lms.start) + self.assertEqual(course.textbooks, []) + self.assertIn('GRADER', course.grading_policy) + self.assertIn('GRADE_CUTOFFS', course.grading_policy) + self.assertGreaterEqual(len(course.checklists), 4) + + # by fetching + module_store = modulestore('direct') + fetched_course = module_store.get_item(course.location) + fetched_item = module_store.get_item(vertical.location) + self.assertIsNotNone(fetched_course.start) + self.assertEqual(course.start, fetched_course.start) + self.assertEqual(fetched_course.start, fetched_item.lms.start) + self.assertEqual(course.textbooks, fetched_course.textbooks) + # is this test too strict? i.e., it requires the dicts to be == + self.assertEqual(course.checklists, fetched_course.checklists) class TemplateTestCase(ModuleStoreTestCase): diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 21d7d69d41..6c23e68240 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -36,7 +36,6 @@ class CourseDetailsTestCase(CourseTestCase): self.assertIsNone(details.enrollment_start, "enrollment_start date somehow initialized " + str(details.enrollment_start)) self.assertIsNone(details.enrollment_end, "enrollment_end date somehow initialized " + str(details.enrollment_end)) self.assertIsNone(details.syllabus, "syllabus somehow initialized" + str(details.syllabus)) - self.assertEqual(details.overview, "", "overview somehow initialized" + details.overview) self.assertIsNone(details.intro_video, "intro_video somehow initialized" + str(details.intro_video)) self.assertIsNone(details.effort, "effort somehow initialized" + str(details.effort)) @@ -49,7 +48,6 @@ class CourseDetailsTestCase(CourseTestCase): self.assertIsNone(jsondetails['enrollment_start'], "enrollment_start date somehow initialized ") self.assertIsNone(jsondetails['enrollment_end'], "enrollment_end date somehow initialized ") self.assertIsNone(jsondetails['syllabus'], "syllabus somehow initialized") - self.assertEqual(jsondetails['overview'], "", "overview somehow initialized") self.assertIsNone(jsondetails['intro_video'], "intro_video somehow initialized") self.assertIsNone(jsondetails['effort'], "effort somehow initialized") diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 7a74e75591..3914892bbf 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -120,7 +120,7 @@ def add_histogram(get_html, module, user): # doesn't like symlinks) filepath = filename data_dir = osfs.root_path.rsplit('/')[-1] - giturl = getattr(module.lms, 'giturl', '') or 'https://github.com/MITx' + giturl = module.lms.giturl or 'https://github.com/MITx' edit_link = "%s/%s/tree/master/%s" % (giturl, data_dir, filepath) else: edit_link = False diff --git a/common/lib/xmodule/xmodule/annotatable_module.py b/common/lib/xmodule/xmodule/annotatable_module.py index e8674360c3..8b1bbc71d3 100644 --- a/common/lib/xmodule/xmodule/annotatable_module.py +++ b/common/lib/xmodule/xmodule/annotatable_module.py @@ -6,12 +6,37 @@ from pkg_resources import resource_string from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xblock.core import Scope, String +import textwrap log = logging.getLogger(__name__) class AnnotatableFields(object): - data = String(help="XML data for the annotation", scope=Scope.content) + data = String(help="XML data for the annotation", scope=Scope.content, + default=textwrap.dedent( + """\ + + +

Enter your (optional) instructions for the exercise in HTML format.

+

Annotations are specified by an <annotation> tag which may may have the following attributes:

+
    +
  • title (optional). Title of the annotation. Defaults to Commentary if omitted.
    • +
  • body (required). Text of the annotation.
    • +
  • problem (optional). Numeric index of the problem associated with this annotation. This is a zero-based index, so the first problem on the page would have problem="0".
    • +
  • highlight (optional). Possible values: yellow, red, orange, green, blue, or purple. Defaults to yellow if this attribute is omitted.
    • +
+ +

Add your HTML with annotation spans here.

+

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Ut sodales laoreet est, egestas gravida felis egestas nec. Aenean at volutpat erat. Cras commodo viverra nibh in aliquam.

+

Nulla facilisi. Pellentesque id vestibulum libero. Suspendisse potenti. Morbi scelerisque nisi vitae felis dictum mattis. Nam sit amet magna elit. Nullam volutpat cursus est, sit amet sagittis odio vulputate et. Curabitur euismod, orci in vulputate imperdiet, augue lorem tempor purus, id aliquet augue turpis a est. Aenean a sagittis libero. Praesent fringilla pretium magna, non condimentum risus elementum nec. Pellentesque faucibus elementum pharetra. Pellentesque vitae metus eros.

+ + """)) + display_name = String( + display_name="Display Name", + help="Display name for this module", + scope=Scope.settings, + default='Annotation', + ) class AnnotatableModule(AnnotatableFields, XModule): diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index eeb8f19439..752f0d3362 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -77,6 +77,14 @@ class CapaFields(object): """ Define the possible fields for a Capa problem """ + display_name = String( + display_name="Display Name", + help="This name appears in the horizontal navigation at the top of the page.", + scope=Scope.settings, + # it'd be nice to have a useful default but it screws up other things; so, + # use display_name_with_default for those + default="Blank Advanced Problem" + ) attempts = Integer(help="Number of attempts taken by the student on this problem", default=0, scope=Scope.user_state) max_attempts = Integer( @@ -94,7 +102,8 @@ class CapaFields(object): display_name="Show Answer", help=("Defines when to show the answer to the problem. " "A default value can be set in Advanced Settings."), - scope=Scope.settings, default="closed", + scope=Scope.settings, + default="closed", values=[ {"display_name": "Always", "value": "always"}, {"display_name": "Answered", "value": "answered"}, @@ -106,21 +115,24 @@ class CapaFields(object): ) force_save_button = Boolean( help="Whether to force the save button to appear on the page", - scope=Scope.settings, default=False + scope=Scope.settings, + default=False ) rerandomize = Randomization( display_name="Randomization", help="Defines how often inputs are randomized when a student loads the problem. " - "This setting only applies to problems that can have randomly generated numeric values. " - "A default value can be set in Advanced Settings.", - default="always", scope=Scope.settings, values=[ + "This setting only applies to problems that can have randomly generated numeric values. " + "A default value can be set in Advanced Settings.", + default="always", + scope=Scope.settings, + values=[ {"display_name": "Always", "value": "always"}, {"display_name": "On Reset", "value": "onreset"}, {"display_name": "Never", "value": "never"}, {"display_name": "Per Student", "value": "per_student"} ] ) - data = String(help="XML data for the problem", scope=Scope.content) + data = String(help="XML data for the problem", scope=Scope.content, default="") correct_map = Dict(help="Dictionary with the correctness of current student answers", scope=Scope.user_state, default={}) input_state = Dict(help="Dictionary for maintaining the state of inputtypes", scope=Scope.user_state) @@ -134,13 +146,12 @@ class CapaFields(object): values={"min": 0, "step": .1}, scope=Scope.settings ) - markdown = String(help="Markdown source of this module", scope=Scope.settings) + markdown = String(help="Markdown source of this module", default="", scope=Scope.settings) source_code = String( help="Source code for LaTeX and Word problems. This feature is not well-supported.", scope=Scope.settings ) - class CapaModule(CapaFields, XModule): """ An XModule implementing LonCapa format problems, implemented by way of diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 52d98f032e..13f00bb77e 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -9,6 +9,7 @@ from xblock.core import Integer, Scope, String, List, Float, Boolean from xmodule.open_ended_grading_classes.combined_open_ended_modulev1 import CombinedOpenEndedV1Module, CombinedOpenEndedV1Descriptor from collections import namedtuple from .fields import Date +import textwrap log = logging.getLogger("mitx.courseware") @@ -27,6 +28,38 @@ VERSION_TUPLES = { } DEFAULT_VERSION = 1 +DEFAULT_DATA = textwrap.dedent("""\ + + + + + Category 1 + + + + + + +

Why is the sky blue?

+ + + + + + + + Enter essay here. + This is the answer. + {"grader_settings" : "peer_grading.conf", "problem_id" : "700x/Demo"} + + + + +""") class VersionInteger(Integer): @@ -85,13 +118,30 @@ class CombinedOpenEndedFields(object): 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) + data = String(help="XML data for the problem", scope=Scope.content, + default=DEFAULT_DATA) 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"} ) - markdown = String(help="Markdown source of this module", scope=Scope.settings) + markdown = String( + help="Markdown source of this module", + default=textwrap.dedent("""\ + [rubric] + + Category 1 + - The response does not incorporate what is needed for a one response. + - The response is correct for category 1. + [rubric] + [prompt] +

Why is the sky blue?

+ [prompt] + [tasks] + (Self), ({1-2}AI) + [tasks] + """), + scope=Scope.settings + ) class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule): diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index d75033c8a0..ceadee1c15 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -145,16 +145,56 @@ class TextbookList(List): class CourseFields(object): - textbooks = TextbookList(help="List of pairs of (title, url) for textbooks used in this course", scope=Scope.content) + textbooks = TextbookList(help="List of pairs of (title, url) for textbooks used in this course", + default=[], scope=Scope.content) wiki_slug = String(help="Slug that points to the wiki for this course", scope=Scope.content) enrollment_start = Date(help="Date that enrollment for this class is opened", scope=Scope.settings) enrollment_end = Date(help="Date that enrollment for this class is closed", scope=Scope.settings) - start = Date(help="Start time when this module is visible", scope=Scope.settings) + start = Date(help="Start time when this module is visible", + # using now(UTC()) resulted in fractional seconds which screwed up comparisons and anyway w/b the + # time of first invocation of this stmt on the server + default=datetime.fromtimestamp(0, UTC()), + scope=Scope.settings) end = Date(help="Date that this class ends", scope=Scope.settings) advertised_start = String(help="Date that this course is advertised to start", scope=Scope.settings) - grading_policy = Dict(help="Grading policy definition for this class", scope=Scope.content) + grading_policy = Dict(help="Grading policy definition for this class", + default={"GRADER": [ + { + "type": "Homework", + "min_count": 12, + "drop_count": 2, + "short_label": "HW", + "weight": 0.15 + }, + { + "type": "Lab", + "min_count": 12, + "drop_count": 2, + "weight": 0.15 + }, + { + "type": "Midterm Exam", + "short_label": "Midterm", + "min_count": 1, + "drop_count": 0, + "weight": 0.3 + }, + { + "type": "Final Exam", + "short_label": "Final", + "min_count": 1, + "drop_count": 0, + "weight": 0.4 + } + ], + "GRADE_CUTOFFS": { + "Pass": 0.5 + }}, + scope=Scope.content) show_calculator = Boolean(help="Whether to show the calculator in this course", default=False, scope=Scope.settings) - display_name = String(help="Display name for this module", scope=Scope.settings) + display_name = String( + help="Display name for this module", default="Empty", + display_name="Display Name", scope=Scope.settings) tabs = List(help="List of tabs to enable in this course", scope=Scope.settings) end_of_course_survey_url = String(help="Url for the end-of-course survey", scope=Scope.settings) discussion_blackouts = List(help="List of pairs of start/end dates for discussion blackouts", scope=Scope.settings) @@ -175,7 +215,125 @@ class CourseFields(object): allow_anonymous_to_peers = Boolean(scope=Scope.settings, default=False) advanced_modules = List(help="Beta modules used in your course", scope=Scope.settings) has_children = True - checklists = List(scope=Scope.settings) + checklists = List(scope=Scope.settings, + default=[ + {"short_description" : "Getting Started With Studio", + "items" : [{"short_description": "Add Course Team Members", + "long_description": "Grant your collaborators permission to edit your course so you can work together.", + "is_checked": False, + "action_url": "ManageUsers", + "action_text": "Edit Course Team", + "action_external": False}, + {"short_description": "Set Important Dates for Your Course", + "long_description": "Establish your course's student enrollment and launch dates on the Schedule and Details page.", + "is_checked": False, + "action_url": "SettingsDetails", + "action_text": "Edit Course Details & Schedule", + "action_external": False}, + {"short_description": "Draft Your Course's Grading Policy", + "long_description": "Set up your assignment types and grading policy even if you haven't created all your assignments.", + "is_checked": False, + "action_url": "SettingsGrading", + "action_text": "Edit Grading Settings", + "action_external": False}, + {"short_description": "Explore the Other Studio Checklists", + "long_description": "Discover other available course authoring tools, and find help when you need it.", + "is_checked": False, + "action_url": "", + "action_text": "", + "action_external": False}] + }, + {"short_description" : "Draft a Rough Course Outline", + "items" : [{"short_description": "Create Your First Section and Subsection", + "long_description": "Use your course outline to build your first Section and Subsection.", + "is_checked": False, + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "action_external": False}, + {"short_description": "Set Section Release Dates", + "long_description": "Specify the release dates for each Section in your course. Sections become visible to students on their release dates.", + "is_checked": False, + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "action_external": False}, + {"short_description": "Designate a Subsection as Graded", + "long_description": "Set a Subsection to be graded as a specific assignment type. Assignments within graded Subsections count toward a student's final grade.", + "is_checked": False, + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "action_external": False}, + {"short_description": "Reordering Course Content", + "long_description": "Use drag and drop to reorder the content in your course.", + "is_checked": False, + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "action_external": False}, + {"short_description": "Renaming Sections", + "long_description": "Rename Sections by clicking the Section name from the Course Outline.", + "is_checked": False, + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "action_external": False}, + {"short_description": "Deleting Course Content", + "long_description": "Delete Sections, Subsections, or Units you don't need anymore. Be careful, as there is no Undo function.", + "is_checked": False, + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "action_external": False}, + {"short_description": "Add an Instructor-Only Section to Your Outline", + "long_description": "Some course authors find using a section for unsorted, in-progress work useful. To do this, create a section and set the release date to the distant future.", + "is_checked": False, + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "action_external": False}] + }, + {"short_description" : "Explore edX's Support Tools", + "items" : [{"short_description": "Explore the Studio Help Forum", + "long_description": "Access the Studio Help forum from the menu that appears when you click your user name in the top right corner of Studio.", + "is_checked": False, + "action_url": "http://help.edge.edx.org/", + "action_text": "Visit Studio Help", + "action_external": True}, + {"short_description": "Enroll in edX 101", + "long_description": "Register for edX 101, edX's primer for course creation.", + "is_checked": False, + "action_url": "https://edge.edx.org/courses/edX/edX101/How_to_Create_an_edX_Course/about", + "action_text": "Register for edX 101", + "action_external": True}, + {"short_description": "Download the Studio Documentation", + "long_description": "Download the searchable Studio reference documentation in PDF form.", + "is_checked": False, + "action_url": "http://files.edx.org/Getting_Started_with_Studio.pdf", + "action_text": "Download Documentation", + "action_external": True}] + }, + {"short_description" : "Draft Your Course About Page", + "items" : [{"short_description": "Draft a Course Description", + "long_description": "Courses on edX have an About page that includes a course video, description, and more. Draft the text students will read before deciding to enroll in your course.", + "is_checked": False, + "action_url": "SettingsDetails", + "action_text": "Edit Course Schedule & Details", + "action_external": False}, + {"short_description": "Add Staff Bios", + "long_description": "Showing prospective students who their instructor will be is helpful. Include staff bios on the course About page.", + "is_checked": False, + "action_url": "SettingsDetails", + "action_text": "Edit Course Schedule & Details", + "action_external": False}, + {"short_description": "Add Course FAQs", + "long_description": "Include a short list of frequently asked questions about your course.", + "is_checked": False, + "action_url": "SettingsDetails", + "action_text": "Edit Course Schedule & Details", + "action_external": False}, + {"short_description": "Add Course Prerequisites", + "long_description": "Let students know what knowledge and/or skills they should have before they enroll in your course.", + "is_checked": False, + "action_url": "SettingsDetails", + "action_text": "Edit Course Schedule & Details", + "action_external": False}] + } + ]) info_sidebar_name = String(scope=Scope.settings, default='Course Handouts') show_timezone = Boolean(help="True if timezones should be shown on dates in the courseware", scope=Scope.settings, default=True) enrollment_domain = String(help="External login method associated with user accounts allowed to register in course", @@ -220,18 +378,16 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): self.wiki_slug = self.location.course msg = None - if self.start is None: - msg = "Course loaded without a valid start date. id = %s" % self.id - self.start = datetime.now(UTC()) - log.critical(msg) - self.system.error_tracker(msg) # NOTE: relies on the modulestore to call set_grading_policy() right after # init. (Modulestore is in charge of figuring out where to load the policy from) # NOTE (THK): This is a last-minute addition for Fall 2012 launch to dynamically # disable the syllabus content for courses that do not provide a syllabus - self.syllabus_present = self.system.resources_fs.exists(path('syllabus')) + if self.system.resources_fs is None: + self.syllabus_present = False + else: + self.syllabus_present = self.system.resources_fs.exists(path('syllabus')) self._grading_policy = {} self.set_grading_policy(self.grading_policy) @@ -252,42 +408,33 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): log.error(msg) continue - def default_grading_policy(self): - """ - Return a dict which is a copy of the default grading policy - """ - return {"GRADER": [ - { - "type": "Homework", - "min_count": 12, - "drop_count": 2, - "short_label": "HW", - "weight": 0.15 - }, - { - "type": "Lab", - "min_count": 12, - "drop_count": 2, - "weight": 0.15 - }, - { - "type": "Midterm Exam", - "short_label": "Midterm", - "min_count": 1, - "drop_count": 0, - "weight": 0.3 - }, - { - "type": "Final Exam", - "short_label": "Final", - "min_count": 1, - "drop_count": 0, - "weight": 0.4 - } - ], - "GRADE_CUTOFFS": { - "Pass": 0.5 - }} + # TODO check that this is still needed here and can't be by defaults. + if self.tabs is None: + # When calling the various _tab methods, can omit the 'type':'blah' from the + # first arg, since that's only used for dispatch + tabs = [] + tabs.append({'type': 'courseware'}) + tabs.append({'type': 'course_info', 'name': 'Course Info'}) + + if self.syllabus_present: + tabs.append({'type': 'syllabus'}) + + tabs.append({'type': 'textbooks'}) + + # # If they have a discussion link specified, use that even if we feature + # # flag discussions off. Disabling that is mostly a server safety feature + # # at this point, and we don't need to worry about external sites. + if self.discussion_link: + tabs.append({'type': 'external_discussion', 'link': self.discussion_link}) + else: + tabs.append({'type': 'discussion', 'name': 'Discussion'}) + + tabs.append({'type': 'wiki', 'name': 'Wiki'}) + + if not self.hide_progress_tab: + tabs.append({'type': 'progress', 'name': 'Progress'}) + + self.tabs = tabs def set_grading_policy(self, course_policy): """ @@ -298,7 +445,13 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): course_policy = {} # Load the global settings as a dictionary - grading_policy = self.default_grading_policy() + grading_policy = self.grading_policy + # BOY DO I HATE THIS grading_policy CODE ACROBATICS YET HERE I ADD MORE (dhm)--this fixes things persisted w/ + # defective grading policy values (but not None) + if 'GRADER' not in grading_policy: + grading_policy['GRADER'] = CourseFields.grading_policy.default['GRADER'] + if 'GRADE_CUTOFFS' not in grading_policy: + grading_policy['GRADE_CUTOFFS'] = CourseFields.grading_policy.default['GRADE_CUTOFFS'] # Override any global settings with the course settings grading_policy.update(course_policy) @@ -354,10 +507,6 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): system.error_tracker("Unable to decode grading policy as json") policy = {} - # cdodge: import the grading policy information that is on disk and put into the - # descriptor 'definition' bucket as a dictionary so that it is persisted in the DB - instance.grading_policy = policy - # now set the current instance. set_grading_policy() will apply some inheritance rules instance.set_grading_policy(policy) @@ -661,6 +810,7 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): if isinstance(self.advertised_start, basestring): return try_parse_iso_8601(self.advertised_start) elif self.advertised_start is None and self.start is None: + # TODO this is an impossible state since the init function forces start to have a value return 'TBD' else: return (self.advertised_start or self.start).strftime("%b %d, %Y") diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py index aef4821839..fac6a498e5 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/common/lib/xmodule/xmodule/discussion_module.py @@ -4,17 +4,27 @@ from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.editing_module import MetadataOnlyEditingDescriptor from xblock.core import String, Scope +from uuid import uuid4 class DiscussionFields(object): - discussion_id = String(scope=Scope.settings) + discussion_id = String(scope=Scope.settings, default="$$GUID$$") + display_name = String( + display_name="Display Name", + help="Display name for this module", + default="Discussion Tag", + scope=Scope.settings) + data = String(help="XML data for the problem", scope=Scope.content, + default="") discussion_category = String( display_name="Category", + default="Week 1", help="A category name for the discussion. This name appears in the left pane of the discussion forum for the course.", scope=Scope.settings ) discussion_target = String( display_name="Subcategory", + default="Topic-Level Student-Visible Label", help="A subcategory name for the discussion. This name appears in the left pane of the discussion forum for the course.", scope=Scope.settings ) @@ -36,9 +46,15 @@ class DiscussionModule(DiscussionFields, XModule): class DiscussionDescriptor(DiscussionFields, MetadataOnlyEditingDescriptor, RawDescriptor): - module_class = DiscussionModule - template_dir_name = "discussion" + def __init__(self, *args, **kwargs): + super(DiscussionDescriptor, self).__init__(*args, **kwargs) + # is this too late? i.e., will it get persisted and stay static w/ the first value + # any code references. I believe so. + if self.discussion_id == '$$GUID$$': + self.discussion_id = uuid4().hex + + module_class = DiscussionModule # The discussion XML format uses `id` and `for` attributes, # but these would overload other module attributes, so we prefix them # for actual use in the code diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 0f7e789906..9ff2e4d9a8 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -13,12 +13,21 @@ from xmodule.html_checker import check_html from xmodule.stringify import stringify_children from xmodule.x_module import XModule from xmodule.xml_module import XmlDescriptor, name_to_pathname +import textwrap log = logging.getLogger("mitx.courseware") class HtmlFields(object): - data = String(help="Html contents to display for this module", scope=Scope.content) + display_name = String( + display_name="Display Name", + help="This name appears in the horizontal navigation at the top of the page.", + scope=Scope.settings, + # it'd be nice to have a useful default but it screws up other things; so, + # use display_name_with_default for those + default="Blank HTML Page" + ) + data = String(help="Html contents to display for this module", default="", scope=Scope.content) source_code = String(help="Source code for LaTeX documents. This feature is not well-supported.", scope=Scope.settings) @@ -32,7 +41,7 @@ class HtmlModule(HtmlFields, XModule): css = {'scss': [resource_string(__name__, 'css/html/display.scss')]} def get_html(self): - if self.system.anonymous_student_id: + if self.system.anonymous_student_id: return self.data.replace("%%USER_ID%%", self.system.anonymous_student_id) return self.data @@ -169,26 +178,88 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor): elt.set("filename", relname) return elt +class AboutFields(object): + display_name = String( + help="Display name for this module", + scope=Scope.settings, + default="overview", + ) + data = String( + help="Html contents to display for this module", + default="", + scope=Scope.content + ) -class AboutDescriptor(HtmlDescriptor): +class AboutModule(AboutFields, HtmlModule): + """ + Overriding defaults but otherwise treated as HtmlModule. + """ + pass + +class AboutDescriptor(AboutFields, HtmlDescriptor): """ These pieces of course content are treated as HtmlModules but we need to overload where the templates are located in order to be able to create new ones """ template_dir_name = "about" + module_class = AboutModule + +class StaticTabFields(object): + """ + The overrides for Static Tabs + """ + display_name = String( + display_name="Display Name", + help="This name appears in the horizontal navigation at the top of the page.", + scope=Scope.settings, + default="Empty", + ) + data = String( + default=textwrap.dedent("""\ +

This is where you can add additional pages to your courseware. Click the 'edit' button to begin editing.

+ """), + scope=Scope.content, + help="HTML for the additional pages" + ) -class StaticTabDescriptor(HtmlDescriptor): +class StaticTabModule(StaticTabFields, HtmlModule): + """ + Supports the field overrides + """ + pass + +class StaticTabDescriptor(StaticTabFields, HtmlDescriptor): """ These pieces of course content are treated as HtmlModules but we need to overload where the templates are located in order to be able to create new ones """ template_dir_name = "statictab" + module_class = StaticTabModule -class CourseInfoDescriptor(HtmlDescriptor): +class CourseInfoFields(object): + """ + Field overrides + """ + data = String( + help="Html contents to display for this module", + default="
    ", + scope=Scope.content + ) + + +class CourseInfoModule(CourseInfoFields, HtmlModule): + """ + Just to support xblock field overrides + """ + pass + + +class CourseInfoDescriptor(CourseInfoFields, HtmlDescriptor): """ These pieces of course content are treated as HtmlModules but we need to overload where the templates are located in order to be able to create new ones """ template_dir_name = "courseinfo" + module_class = CourseInfoModule diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index 7df444a892..c88a2e1b38 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -59,6 +59,15 @@ class PeerGradingFields(object): 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"} ) + display_name = String( + display_name="Display Name", + help="Display name for this module", + scope=Scope.settings, + default="Peer Grading Interface" + ) + data = String(help="Html contents to display for this module", + default='', + scope=Scope.content) class PeerGradingModule(PeerGradingFields, XModule): diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py index 554be73926..4c6c719224 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -13,7 +13,7 @@ class RawDescriptor(XmlDescriptor, XMLEditingDescriptor): Module that provides a raw editing view of its data and children. It requires that the definition xml is valid. """ - data = String(help="XML data for the module", scope=Scope.content) + data = String(help="XML data for the module", default="", scope=Scope.content) @classmethod def definition_from_xml(cls, xml_object, system): diff --git a/common/lib/xmodule/xmodule/templates/about/empty.yaml b/common/lib/xmodule/xmodule/templates/about/empty.yaml index fa3ed606bd..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/about/empty.yaml +++ b/common/lib/xmodule/xmodule/templates/about/empty.yaml @@ -1,5 +1 @@ ---- -metadata: - display_name: Empty -data: "

    This is where you can add additional information about your course.

    " -children: [] \ No newline at end of file +{} diff --git a/common/lib/xmodule/xmodule/templates/about/overview.yaml b/common/lib/xmodule/xmodule/templates/about/overview.yaml index 0031ebffaf..9b2e895526 100644 --- a/common/lib/xmodule/xmodule/templates/about/overview.yaml +++ b/common/lib/xmodule/xmodule/templates/about/overview.yaml @@ -3,51 +3,50 @@ metadata: display_name: overview data: | -
    -

    About This Course

    -

    Include your long course description here. The long course description should contain 150-400 words.

    +
    +

    About This Course

    +

    Include your long course description here. The long course description should contain 150-400 words.

    -

    This is paragraph 2 of the long course description. Add more paragraphs as needed. Make sure to enclose them in paragraph tags.

    -
    +

    This is paragraph 2 of the long course description. Add more paragraphs as needed. Make sure to enclose them in paragraph tags.

    +
    -
    -

    Prerequisites

    -

    Add information about course prerequisites here.

    -
    +
    +

    Prerequisites

    +

    Add information about course prerequisites here.

    +
    -
    -

    Course Staff

    -
    -
    - -
    +
    +

    Course Staff

    +
    +
    + +
    -

    Staff Member #1

    -

    Biography of instructor/staff member #1

    -
    +

    Staff Member #1

    +

    Biography of instructor/staff member #1

    +
    -
    -
    - -
    +
    +
    + +
    -

    Staff Member #2

    -

    Biography of instructor/staff member #2

    -
    -
    +

    Staff Member #2

    +

    Biography of instructor/staff member #2

    + + -
    -
    -

    Frequently Asked Questions

    -
    -

    Do I need to buy a textbook?

    -

    No, a free online version of Chemistry: Principles, Patterns, and Applications, First Edition by Bruce Averill and Patricia Eldredge will be available, though you can purchase a printed version (published by FlatWorld Knowledge) if you’d like.

    -
    +
    +
    +

    Frequently Asked Questions

    +
    +

    Do I need to buy a textbook?

    +

    No, a free online version of Chemistry: Principles, Patterns, and Applications, First Edition by Bruce Averill and Patricia Eldredge will be available, though you can purchase a printed version (published by FlatWorld Knowledge) if you’d like.

    +
    -
    -

    Question #2

    -

    Your answer would be displayed here.

    -
    -
    -
    -children: [] +
    +

    Question #2

    +

    Your answer would be displayed here.

    +
    +
    +
    diff --git a/common/lib/xmodule/xmodule/templates/annotatable/default.yaml b/common/lib/xmodule/xmodule/templates/annotatable/default.yaml index 31dd489fb4..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/annotatable/default.yaml +++ b/common/lib/xmodule/xmodule/templates/annotatable/default.yaml @@ -1,20 +1 @@ ---- -metadata: - display_name: 'Annotation' -data: | - - -

    Enter your (optional) instructions for the exercise in HTML format.

    -

    Annotations are specified by an <annotation> tag which may may have the following attributes:

    -
      -
    • title (optional). Title of the annotation. Defaults to Commentary if omitted.
      • -
    • body (required). Text of the annotation.
      • -
    • problem (optional). Numeric index of the problem associated with this annotation. This is a zero-based index, so the first problem on the page would have problem="0".
      • -
    • highlight (optional). Possible values: yellow, red, orange, green, blue, or purple. Defaults to yellow if this attribute is omitted.
      • -
    -     -

    Add your HTML with annotation spans here.

    -

    Lorem ipsum dolor sit amet, consectetur adipiscing elit. Ut sodales laoreet est, egestas gravida felis egestas nec. Aenean at volutpat erat. Cras commodo viverra nibh in aliquam.

    -

    Nulla facilisi. Pellentesque id vestibulum libero. Suspendisse potenti. Morbi scelerisque nisi vitae felis dictum mattis. Nam sit amet magna elit. Nullam volutpat cursus est, sit amet sagittis odio vulputate et. Curabitur euismod, orci in vulputate imperdiet, augue lorem tempor purus, id aliquet augue turpis a est. Aenean a sagittis libero. Praesent fringilla pretium magna, non condimentum risus elementum nec. Pellentesque faucibus elementum pharetra. Pellentesque vitae metus eros.

    -     -children: [] +{} diff --git a/common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml b/common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml index f7d639ebfb..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml +++ b/common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml @@ -1,37 +1 @@ ---- -metadata: - display_name: Open Ended Response - markdown: "" -data: | - - - - - Category 1 - - - - - - -

    Why is the sky blue?

    -     - - - - - - - Enter essay here. - This is the answer. - {"grader_settings" : "peer_grading.conf", "problem_id" : "700x/Demo"} - - - -     - -children: [] +{} diff --git a/common/lib/xmodule/xmodule/templates/course/empty.yaml b/common/lib/xmodule/xmodule/templates/course/empty.yaml index 89f1bfcf21..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/course/empty.yaml +++ b/common/lib/xmodule/xmodule/templates/course/empty.yaml @@ -1,124 +1 @@ ---- -metadata: - display_name: Empty - start: 2020-10-10T10:00 - checklists: [ - {"short_description" : "Getting Started With Studio", - "items" : [{"short_description": "Add Course Team Members", - "long_description": "Grant your collaborators permission to edit your course so you can work together.", - "is_checked": false, - "action_url": "ManageUsers", - "action_text": "Edit Course Team", - "action_external": false}, - {"short_description": "Set Important Dates for Your Course", - "long_description": "Establish your course's student enrollment and launch dates on the Schedule and Details page.", - "is_checked": false, - "action_url": "SettingsDetails", - "action_text": "Edit Course Details & Schedule", - "action_external": false}, - {"short_description": "Draft Your Course's Grading Policy", - "long_description": "Set up your assignment types and grading policy even if you haven't created all your assignments.", - "is_checked": false, - "action_url": "SettingsGrading", - "action_text": "Edit Grading Settings", - "action_external": false}, - {"short_description": "Explore the Other Studio Checklists", - "long_description": "Discover other available course authoring tools, and find help when you need it.", - "is_checked": false, - "action_url": "", - "action_text": "", - "action_external": false}] - }, - {"short_description" : "Draft a Rough Course Outline", - "items" : [{"short_description": "Create Your First Section and Subsection", - "long_description": "Use your course outline to build your first Section and Subsection.", - "is_checked": false, - "action_url": "CourseOutline", - "action_text": "Edit Course Outline", - "action_external": false}, - {"short_description": "Set Section Release Dates", - "long_description": "Specify the release dates for each Section in your course. Sections become visible to students on their release dates.", - "is_checked": false, - "action_url": "CourseOutline", - "action_text": "Edit Course Outline", - "action_external": false}, - {"short_description": "Designate a Subsection as Graded", - "long_description": "Set a Subsection to be graded as a specific assignment type. Assignments within graded Subsections count toward a student's final grade.", - "is_checked": false, - "action_url": "CourseOutline", - "action_text": "Edit Course Outline", - "action_external": false}, - {"short_description": "Reordering Course Content", - "long_description": "Use drag and drop to reorder the content in your course.", - "is_checked": false, - "action_url": "CourseOutline", - "action_text": "Edit Course Outline", - "action_external": false}, - {"short_description": "Renaming Sections", - "long_description": "Rename Sections by clicking the Section name from the Course Outline.", - "is_checked": false, - "action_url": "CourseOutline", - "action_text": "Edit Course Outline", - "action_external": false}, - {"short_description": "Deleting Course Content", - "long_description": "Delete Sections, Subsections, or Units you don't need anymore. Be careful, as there is no Undo function.", - "is_checked": false, - "action_url": "CourseOutline", - "action_text": "Edit Course Outline", - "action_external": false}, - {"short_description": "Add an Instructor-Only Section to Your Outline", - "long_description": "Some course authors find using a section for unsorted, in-progress work useful. To do this, create a section and set the release date to the distant future.", - "is_checked": false, - "action_url": "CourseOutline", - "action_text": "Edit Course Outline", - "action_external": false}] - }, - {"short_description" : "Explore edX's Support Tools", - "items" : [{"short_description": "Explore the Studio Help Forum", - "long_description": "Access the Studio Help forum from the menu that appears when you click your user name in the top right corner of Studio.", - "is_checked": false, - "action_url": "http://help.edge.edx.org/", - "action_text": "Visit Studio Help", - "action_external": true}, - {"short_description": "Enroll in edX 101", - "long_description": "Register for edX 101, edX's primer for course creation.", - "is_checked": false, - "action_url": "https://edge.edx.org/courses/edX/edX101/How_to_Create_an_edX_Course/about", - "action_text": "Register for edX 101", - "action_external": true}, - {"short_description": "Download the Studio Documentation", - "long_description": "Download the searchable Studio reference documentation in PDF form.", - "is_checked": false, - "action_url": "http://files.edx.org/Getting_Started_with_Studio.pdf", - "action_text": "Download Documentation", - "action_external": true}] - }, - {"short_description" : "Draft Your Course About Page", - "items" : [{"short_description": "Draft a Course Description", - "long_description": "Courses on edX have an About page that includes a course video, description, and more. Draft the text students will read before deciding to enroll in your course.", - "is_checked": false, - "action_url": "SettingsDetails", - "action_text": "Edit Course Schedule & Details", - "action_external": false}, - {"short_description": "Add Staff Bios", - "long_description": "Showing prospective students who their instructor will be is helpful. Include staff bios on the course About page.", - "is_checked": false, - "action_url": "SettingsDetails", - "action_text": "Edit Course Schedule & Details", - "action_external": false}, - {"short_description": "Add Course FAQs", - "long_description": "Include a short list of frequently asked questions about your course.", - "is_checked": false, - "action_url": "SettingsDetails", - "action_text": "Edit Course Schedule & Details", - "action_external": false}, - {"short_description": "Add Course Prerequisites", - "long_description": "Let students know what knowledge and/or skills they should have before they enroll in your course.", - "is_checked": false, - "action_url": "SettingsDetails", - "action_text": "Edit Course Schedule & Details", - "action_external": false}] - } - ] -data: { 'textbooks' : [ ], 'wiki_slug' : null } -children: [] +{} diff --git a/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml b/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml index c6958ed887..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml +++ b/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml @@ -1,5 +1 @@ ---- -metadata: - display_name: Empty -data: "
      " -children: [] \ No newline at end of file +{} diff --git a/common/lib/xmodule/xmodule/templates/default/empty.yaml b/common/lib/xmodule/xmodule/templates/default/empty.yaml index a2fb2b5832..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/default/empty.yaml +++ b/common/lib/xmodule/xmodule/templates/default/empty.yaml @@ -1,5 +1 @@ ---- -metadata: - display_name: Empty -data: "" -children: [] +{} diff --git a/common/lib/xmodule/xmodule/templates/discussion/default.yaml b/common/lib/xmodule/xmodule/templates/discussion/default.yaml index 049e34b3e7..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/discussion/default.yaml +++ b/common/lib/xmodule/xmodule/templates/discussion/default.yaml @@ -1,9 +1 @@ ---- -metadata: - display_name: Discussion Tag - for: Topic-Level Student-Visible Label - id: $$GUID$$ - discussion_category: Week 1 -data: | - -children: [] +{} diff --git a/common/lib/xmodule/xmodule/templates/html/announcement.yaml b/common/lib/xmodule/xmodule/templates/html/announcement.yaml index 82fe8fbc03..30a8ccb41e 100644 --- a/common/lib/xmodule/xmodule/templates/html/announcement.yaml +++ b/common/lib/xmodule/xmodule/templates/html/announcement.yaml @@ -1,7 +1,6 @@ --- metadata: - display_name: Announcement - + display_name: Announcement data: |
      1. diff --git a/common/lib/xmodule/xmodule/templates/html/empty.yaml b/common/lib/xmodule/xmodule/templates/html/empty.yaml index 40b005af28..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/html/empty.yaml +++ b/common/lib/xmodule/xmodule/templates/html/empty.yaml @@ -1,7 +1 @@ ---- -metadata: - display_name: Blank HTML Page - -data: | - -children: [] \ No newline at end of file +{} diff --git a/common/lib/xmodule/xmodule/templates/html/latex_html.yaml b/common/lib/xmodule/xmodule/templates/html/latex_html.yaml index ff92f2aead..ba5c4b5c06 100644 --- a/common/lib/xmodule/xmodule/templates/html/latex_html.yaml +++ b/common/lib/xmodule/xmodule/templates/html/latex_html.yaml @@ -1,16 +1,16 @@ --- metadata: display_name: E-text Written in LaTeX - source_code: | - \subsection{Example of E-text in LaTeX} +source_code: | + \subsection{Example of E-text in LaTeX} - It is very convenient to write complex equations in LaTeX. + It is very convenient to write complex equations in LaTeX. - \begin{equation} - x = \frac{-b\pm\sqrt{b^2-4*a*c}}{2a} - \end{equation} + \begin{equation} + x = \frac{-b\pm\sqrt{b^2-4*a*c}}{2a} + \end{equation} - Seize the moment. + Seize the moment. data: | diff --git a/common/lib/xmodule/xmodule/templates/peer_grading/default.yaml b/common/lib/xmodule/xmodule/templates/peer_grading/default.yaml index 5d88a18ad8..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/peer_grading/default.yaml +++ b/common/lib/xmodule/xmodule/templates/peer_grading/default.yaml @@ -1,9 +1 @@ ---- -metadata: - display_name: Peer Grading Interface - max_grade: 1 -data: | - - - -children: [] +{} diff --git a/common/lib/xmodule/xmodule/templates/problem/blank_common.yaml b/common/lib/xmodule/xmodule/templates/problem/blank_common.yaml new file mode 100644 index 0000000000..3dcac29aba --- /dev/null +++ b/common/lib/xmodule/xmodule/templates/problem/blank_common.yaml @@ -0,0 +1,5 @@ +--- +metadata: + display_name: Blank Common Problem + markdown: "" +data: "" diff --git a/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml b/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml index 56f802a6a3..3b051f2ba8 100644 --- a/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml @@ -5,59 +5,58 @@ metadata: rerandomize: never showanswer: finished data: | - - Please make a voltage divider that splits the provided voltage evenly. - - -
        - -
        - - dc_value = "dc analysis not found" - for response in submission[0]: - if response[0] == 'dc': - for node in response[1:]: - dc_value = node['output'] - - if dc_value == .5: - correct = ['correct'] - else: - correct = ['incorrect'] - -         - -

        Make a high pass filter

        -
        - -
        - - ac_values = None - for response in submission[0]: - if response[0] == 'ac': - for node in response[1:]: - ac_values = node['NodeA'] - print "the ac analysis value:", ac_values - if ac_values == None: - correct = ['incorrect'] - elif ac_values[0][1] < ac_values[1][1]: - correct = ['correct'] - else: - correct = ['incorrect'] - -         - - -
        -

        Explanation

        -

        A voltage divider that evenly divides the input voltage can be formed with two identically valued resistors, with the sampled voltage taken in between the two.

        -

        -

        A simple high-pass filter without any further constaints can be formed by simply putting a resister in series with a capacitor. The actual values of the components do not really matter in order to meet the constraints of the problem.

        -

        -
        -         -         -children: [] + + Please make a voltage divider that splits the provided voltage evenly. + + +
        + +
        + + dc_value = "dc analysis not found" + for response in submission[0]: + if response[0] == 'dc': + for node in response[1:]: + dc_value = node['output'] + + if dc_value == .5: + correct = ['correct'] + else: + correct = ['incorrect'] + +         + +

        Make a high pass filter

        +
        + +
        + + ac_values = None + for response in submission[0]: + if response[0] == 'ac': + for node in response[1:]: + ac_values = node['NodeA'] + print "the ac analysis value:", ac_values + if ac_values == None: + correct = ['incorrect'] + elif ac_values[0][1] < ac_values[1][1]: + correct = ['correct'] + else: + correct = ['incorrect'] + +         + + +
        +

        Explanation

        +

        A voltage divider that evenly divides the input voltage can be formed with two identically valued resistors, with the sampled voltage taken in between the two.

        +

        +

        A simple high-pass filter without any further constaints can be formed by simply putting a resister in series with a capacitor. The actual values of the components do not really matter in order to meet the constraints of the problem.

        +

        +
        +         +         diff --git a/common/lib/xmodule/xmodule/templates/problem/empty.yaml b/common/lib/xmodule/xmodule/templates/problem/empty.yaml index 97a2aef423..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/problem/empty.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/empty.yaml @@ -1,11 +1 @@ ---- -metadata: - display_name: Blank Common Problem - rerandomize: never - showanswer: finished - markdown: "" -data: | - - - -children: [] +{} diff --git a/common/lib/xmodule/xmodule/templates/statictab/empty.yaml b/common/lib/xmodule/xmodule/templates/statictab/empty.yaml index 410e1496c2..9e26dfeeb6 100644 --- a/common/lib/xmodule/xmodule/templates/statictab/empty.yaml +++ b/common/lib/xmodule/xmodule/templates/statictab/empty.yaml @@ -1,5 +1 @@ ---- -metadata: - display_name: Empty -data: "

        This is where you can add additional pages to your courseware. Click the 'edit' button to begin editing.

        " -children: [] \ No newline at end of file +{} \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/templates/video/default.yaml b/common/lib/xmodule/xmodule/templates/video/default.yaml index 048e7396c7..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/video/default.yaml +++ b/common/lib/xmodule/xmodule/templates/video/default.yaml @@ -1,5 +1 @@ ---- -metadata: - display_name: default -data: "" -children: [] +{} diff --git a/common/lib/xmodule/xmodule/templates/videoalpha/default.yaml b/common/lib/xmodule/xmodule/templates/videoalpha/default.yaml index 1c25b272a3..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/videoalpha/default.yaml +++ b/common/lib/xmodule/xmodule/templates/videoalpha/default.yaml @@ -1,11 +1 @@ ---- -metadata: - display_name: Video Alpha - version: 1 -data: | - - - - - -children: [] +{} diff --git a/common/lib/xmodule/xmodule/templates/word_cloud/default.yaml b/common/lib/xmodule/xmodule/templates/word_cloud/default.yaml index 53e9eeaae4..0967ef424b 100644 --- a/common/lib/xmodule/xmodule/templates/word_cloud/default.yaml +++ b/common/lib/xmodule/xmodule/templates/word_cloud/default.yaml @@ -1,5 +1 @@ ---- -metadata: - display_name: Word cloud -data: {} -children: [] +{} diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index 3c6203107d..ebff888f34 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -21,6 +21,17 @@ log = logging.getLogger(__name__) class VideoFields(object): """Fields for `VideoModule` and `VideoDescriptor`.""" + display_name = String( + display_name="Display Name", + help="This name appears in the horizontal navigation at the top of the page.", + scope=Scope.settings, + # it'd be nice to have a useful default but it screws up other things; so, + # use display_name_with_default for those + default="Video Title" + ) + data = String(help="XML data for the problem", + default='', + scope=Scope.content) position = Integer(help="Current position in the video", scope=Scope.user_state, default=0) show_captions = Boolean(help="This controls whether or not captions are shown by default.", display_name="Show Captions", scope=Scope.settings, default=True) youtube_id_1_0 = String(help="This is the Youtube ID reference for the normal speed video.", display_name="Default Speed", scope=Scope.settings, default="OEoXaMPEzfM") @@ -129,6 +140,10 @@ def _parse_video_xml(video, xml_data): display_name = xml.get('display_name') if display_name: video.display_name = display_name + elif video.url_name is not None: + # copies the logic of display_name_with_default in order that studio created videos will have an + # initial non guid name + video.display_name = video.url_name.replace('_', ' ') youtube = xml.get('youtube') if youtube: diff --git a/common/lib/xmodule/xmodule/videoalpha_module.py b/common/lib/xmodule/xmodule/videoalpha_module.py index 3b5b90e674..33945c33fc 100644 --- a/common/lib/xmodule/xmodule/videoalpha_module.py +++ b/common/lib/xmodule/xmodule/videoalpha_module.py @@ -28,15 +28,27 @@ from xblock.core import Integer, Scope, String import datetime import time +import textwrap log = logging.getLogger(__name__) class VideoAlphaFields(object): """Fields for `VideoAlphaModule` and `VideoAlphaDescriptor`.""" - data = String(help="XML data for the problem", scope=Scope.content) + data = String(help="XML data for the problem", + default=textwrap.dedent('''\ + + + + + '''), + scope=Scope.content) position = Integer(help="Current position in the video", scope=Scope.user_state, default=0) - display_name = String(help="Display name for this module", scope=Scope.settings) + display_name = String( + display_name="Display Name", help="Display name for this module", + default="Video Alpha", + scope=Scope.settings + ) class VideoAlphaModule(VideoAlphaFields, XModule): diff --git a/common/lib/xmodule/xmodule/word_cloud_module.py b/common/lib/xmodule/xmodule/word_cloud_module.py index a7f3f92795..004e6ed320 100644 --- a/common/lib/xmodule/xmodule/word_cloud_module.py +++ b/common/lib/xmodule/xmodule/word_cloud_module.py @@ -14,7 +14,7 @@ from xmodule.raw_module import RawDescriptor from xmodule.editing_module import MetadataOnlyEditingDescriptor from xmodule.x_module import XModule -from xblock.core import Scope, Dict, Boolean, List, Integer +from xblock.core import Scope, Dict, Boolean, List, Integer, String log = logging.getLogger(__name__) @@ -31,6 +31,12 @@ def pretty_bool(value): class WordCloudFields(object): """XFields for word cloud.""" + display_name = String( + display_name="Display Name", + help="Display name for this module", + scope=Scope.settings, + default="Word cloud" + ) num_inputs = Integer( display_name="Inputs", help="Number of text boxes available for students to input words/sentences.", @@ -234,7 +240,7 @@ class WordCloudModule(WordCloudFields, XModule): return self.content -class WordCloudDescriptor(MetadataOnlyEditingDescriptor, RawDescriptor, WordCloudFields): +class WordCloudDescriptor(WordCloudFields, MetadataOnlyEditingDescriptor, RawDescriptor): """Descriptor for WordCloud Xmodule.""" module_class = WordCloudModule template_dir_name = 'word_cloud' diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 0f5bbf4f2e..aee8e26171 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -7,8 +7,8 @@ from lxml import etree from collections import namedtuple from pkg_resources import resource_listdir, resource_string, resource_isdir -from xmodule.modulestore import Location -from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.modulestore import inheritance, Location +from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecificationError from xblock.core import XBlock, Scope, String, Integer, Float, ModelType @@ -101,6 +101,8 @@ class XModuleFields(object): display_name="Display Name", help="This name appears in the horizontal navigation at the top of the page.", scope=Scope.settings, + # it'd be nice to have a useful default but it screws up other things; so, + # use display_name_with_default for those default=None ) @@ -113,6 +115,14 @@ class XModuleFields(object): scope=Scope.content, default=Location(None), ) + # Please note that in order to be compatible with XBlocks more generally, + # the LMS and CMS shouldn't be using this field. It's only for internal + # consumption by the XModules themselves + category = String( + display_name="xmodule category", + help="This is the category id for the XModule. It's for internal use only", + scope=Scope.content, + ) class XModule(XModuleFields, HTMLSnippet, XBlock): @@ -148,8 +158,16 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): self._model_data = model_data self.system = runtime self.descriptor = descriptor - self.url_name = self.location.name - self.category = self.location.category + # LMS tests don't require descriptor but really it's required + if descriptor: + self.url_name = descriptor.url_name + # don't need to set category as it will automatically get from descriptor + elif isinstance(self.location, Location): + self.url_name = self.location.name + if not hasattr(self, 'category'): + self.category = self.location.category + else: + raise InsufficientSpecificationError() self._loaded_children = None @property @@ -290,36 +308,67 @@ Template = namedtuple("Template", "metadata data children") class ResourceTemplates(object): + """ + Gets the templates associated w/ a containing cls. The cls must have a 'template_dir_name' attribute. + It finds the templates as directly in this directory under 'templates'. + """ @classmethod def templates(cls): """ - Returns a list of Template objects that describe possible templates that can be used - to create a module of this type. - If no templates are provided, there will be no way to create a module of - this type + Returns a list of dictionary field: value objects that describe possible templates that can be used + to seed a module of this type. Expects a class attribute template_dir_name that defines the directory inside the 'templates' resource directory to pull templates from """ templates = [] - dirname = os.path.join('templates', cls.template_dir_name) - if not resource_isdir(__name__, dirname): - log.warning("No resource directory {dir} found when loading {cls_name} templates".format( - dir=dirname, - cls_name=cls.__name__, - )) - return [] - - for template_file in resource_listdir(__name__, dirname): - if not template_file.endswith('.yaml'): - log.warning("Skipping unknown template file %s" % template_file) - continue - template_content = resource_string(__name__, os.path.join(dirname, template_file)) - template = yaml.safe_load(template_content) - templates.append(Template(**template)) + dirname = cls.get_template_dir() + if dirname is not None: + for template_file in resource_listdir(__name__, dirname): + if not template_file.endswith('.yaml'): + log.warning("Skipping unknown template file %s", template_file) + continue + template_content = resource_string(__name__, os.path.join(dirname, template_file)) + template = yaml.safe_load(template_content) + template['template_id'] = template_file + templates.append(template) return templates + @classmethod + def get_template_dir(cls): + if getattr(cls, 'template_dir_name', None): + dirname = os.path.join('templates', getattr(cls, 'template_dir_name')) + if not resource_isdir(__name__, dirname): + log.warning("No resource directory {dir} found when loading {cls_name} templates".format( + dir=dirname, + cls_name=cls.__name__, + )) + return None + else: + return dirname + else: + return None + + @classmethod + def get_template(cls, template_id): + """ + Get a single template by the given id (which is the file name identifying it w/in the class's + template_dir_name) + + """ + dirname = cls.get_template_dir() + if dirname is not None: + try: + template_content = resource_string(__name__, os.path.join(dirname, template_id)) + except IOError: + return None + template = yaml.safe_load(template_content) + template['template_id'] = template_id + return template + else: + return None + class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): """ @@ -346,9 +395,6 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): # be equal equality_attributes = ('_model_data', 'location') - # Name of resource directory to load templates from - template_dir_name = "default" - # Class level variable # True if this descriptor always requires recalculation of grades, for @@ -386,8 +432,12 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): """ super(XModuleDescriptor, self).__init__(*args, **kwargs) self.system = self.runtime - self.url_name = self.location.name - self.category = self.location.category + if isinstance(self.location, Location): + self.url_name = self.location.name + if not hasattr(self, 'category'): + self.category = self.location.category + else: + raise InsufficientSpecificationError() self._child_instances = None @property @@ -419,11 +469,14 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): if self._child_instances is None: self._child_instances = [] for child_loc in self.children: - try: - child = self.system.load_item(child_loc) - except ItemNotFoundError: - log.exception('Unable to load item {loc}, skipping'.format(loc=child_loc)) - continue + if isinstance(child_loc, XModuleDescriptor): + child = child_loc + else: + try: + child = self.system.load_item(child_loc) + except ItemNotFoundError: + log.exception('Unable to load item {loc}, skipping'.format(loc=child_loc)) + continue self._child_instances.append(child) return self._child_instances @@ -591,6 +644,13 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): """ return [('{}', '{}')] + @property + def xblock_kvs(self): + """ + Use w/ caution. Really intended for use by the persistence layer. + """ + return self._model_data._kvs + # =============================== BUILTIN METHODS ========================== def __eq__(self, other): eq = (self.__class__ == other.__class__ and diff --git a/lms/xmodule_namespace.py b/lms/xmodule_namespace.py index aaef0b76db..a78e27e5af 100644 --- a/lms/xmodule_namespace.py +++ b/lms/xmodule_namespace.py @@ -3,6 +3,8 @@ Namespace that defines fields common to all blocks used in the LMS """ from xblock.core import Namespace, Boolean, Scope, String, Float from xmodule.fields import Date, Timedelta +from datetime import datetime +from pytz import UTC class LmsNamespace(Namespace): @@ -25,7 +27,11 @@ class LmsNamespace(Namespace): scope=Scope.settings, ) - start = Date(help="Start time when this module is visible", scope=Scope.settings) + start = Date( + help="Start time when this module is visible", + default=datetime.fromtimestamp(0, UTC), + scope=Scope.settings + ) due = Date(help="Date that this problem is due by", scope=Scope.settings) source_file = String(help="source file name (eg for latex)", scope=Scope.settings) giturl = String(help="url root for course data git repository", scope=Scope.settings) From 3722685e1a185ff4a0a085bb380ed793cf6d1e59 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Mon, 24 Jun 2013 15:44:49 -0400 Subject: [PATCH 24/73] No longer persist XModule templates Instead, we use XModule field default values when creating an empty XModule. Driven by this use case, we also allow for XModules to be created in memory without being persisted to the database at all. This necessitates a change to the Modulestore api, replacing clone_item with create_draft and save_xmodule. --- CHANGELOG.rst | 7 + README.md | 1 - .../contentstore/course_info_model.py | 4 +- .../contentstore/features/common.py | 2 +- .../component_settings_editor_helpers.py | 16 +- .../features/discussion-editor.py | 4 +- .../contentstore/features/html-editor.py | 2 +- .../contentstore/features/problem-editor.py | 2 +- .../features/studio-overview-togglesection.py | 8 +- cms/djangoapps/contentstore/features/video.py | 2 +- .../management/commands/update_templates.py | 10 - .../contentstore/module_info_model.py | 24 +-- .../contentstore/tests/test_contentstore.py | 108 ++++------ .../tests/test_course_settings.py | 1 + .../contentstore/tests/test_i18n.py | 1 - .../contentstore/tests/test_item.py | 196 +++++++++++++++++- cms/djangoapps/contentstore/tests/utils.py | 1 - cms/djangoapps/contentstore/utils.py | 8 +- .../contentstore/views/checklist.py | 6 +- .../contentstore/views/component.py | 65 ++++-- cms/djangoapps/contentstore/views/course.py | 34 ++- cms/djangoapps/contentstore/views/item.py | 75 ++++--- cms/djangoapps/contentstore/views/user.py | 2 +- .../models/settings/course_grading.py | 10 +- .../coffee/src/views/module_edit.coffee | 17 +- cms/static/coffee/src/views/tabs.coffee | 4 +- cms/static/coffee/src/views/unit.coffee | 4 +- cms/static/js/base.js | 22 +- cms/templates/index.html | 4 +- cms/templates/overview.html | 12 +- cms/templates/unit.html | 99 +++++---- cms/templates/widgets/units.html | 2 +- cms/urls.py | 2 +- common/djangoapps/terrain/course_helpers.py | 2 - common/djangoapps/tests.py | 6 +- common/lib/xmodule/xmodule/abtest_module.py | 2 - .../lib/xmodule/xmodule/annotatable_module.py | 1 - .../xmodule/combined_open_ended_module.py | 1 - common/lib/xmodule/xmodule/course_module.py | 2 - common/lib/xmodule/xmodule/error_module.py | 7 +- common/lib/xmodule/xmodule/foldit_module.py | 1 - common/lib/xmodule/xmodule/gst_module.py | 1 - common/lib/xmodule/xmodule/html_module.py | 4 +- .../spec/combinedopenended/edit_spec.coffee | 4 +- .../xmodule/js/spec/problem/edit_spec.coffee | 4 +- .../js/src/combinedopenended/edit.coffee | 3 +- .../xmodule/js/src/problem/edit.coffee | 5 +- .../xmodule/xmodule/modulestore/__init__.py | 9 +- .../xmodule/xmodule/modulestore/mongo/base.py | 173 ++++++++++++---- .../xmodule/modulestore/mongo/draft.py | 58 ++++-- .../xmodule/modulestore/tests/django_utils.py | 20 -- .../xmodule/modulestore/tests/factories.py | 120 +++-------- .../xmodule/modulestore/tests/test_mongo.py | 4 +- common/lib/xmodule/xmodule/modulestore/xml.py | 5 +- .../combined_open_ended_modulev1.py | 1 - .../open_ended_module.py | 1 - .../self_assessment_module.py | 1 - .../xmodule/xmodule/peer_grading_module.py | 1 - common/lib/xmodule/xmodule/poll_module.py | 1 - common/lib/xmodule/xmodule/templates.py | 96 +-------- .../xmodule/templates/about/empty.yaml | 1 - .../templates/annotatable/default.yaml | 1 - .../templates/combinedopenended/default.yaml | 1 - .../xmodule/templates/course/empty.yaml | 1 - .../xmodule/templates/courseinfo/empty.yaml | 1 - .../xmodule/templates/default/empty.yaml | 1 - .../xmodule/templates/discussion/default.yaml | 1 - .../xmodule/templates/html/announcement.yaml | 1 - .../xmodule/xmodule/templates/html/empty.yaml | 1 - .../xmodule/templates/html/everything.yaml | 33 --- .../xmodule/templates/html/latex_html.yaml | 1 - .../templates/peer_grading/default.yaml | 1 - .../templates/problem/circuitschematic.yaml | 2 +- .../templates/problem/customgrader.yaml | 77 ++++--- .../xmodule/templates/problem/empty.yaml | 1 - .../templates/problem/emptyadvanced.yaml | 10 - .../templates/problem/forumularesponse.yaml | 3 +- .../templates/problem/imageresponse.yaml | 4 +- .../templates/problem/latex_problem.yaml | 2 +- .../templates/problem/multiplechoice.yaml | 13 +- .../templates/problem/numericalresponse.yaml | 22 +- .../templates/problem/optionresponse.yaml | 11 +- .../templates/problem/problem_with_hint.yaml | 3 +- .../templates/problem/string_response.yaml | 11 +- .../templates/sequence/with_video.yaml | 7 - .../xmodule/templates/statictab/empty.yaml | 1 - .../xmodule/templates/video/default.yaml | 1 - .../xmodule/templates/videoalpha/default.yaml | 1 - .../xmodule/templates/word_cloud/default.yaml | 1 - .../lib/xmodule/xmodule/tests/test_logic.py | 3 +- .../xmodule/xmodule/tests/test_xml_module.py | 1 + common/lib/xmodule/xmodule/video_module.py | 1 - .../lib/xmodule/xmodule/videoalpha_module.py | 1 - common/lib/xmodule/xmodule/xml_module.py | 1 + lms/djangoapps/courseware/features/common.py | 5 +- .../courseware/features/navigation.py | 11 +- .../courseware/features/problems.py | 2 +- .../courseware/features/problems_setup.py | 6 +- lms/djangoapps/courseware/features/video.py | 7 +- lms/djangoapps/courseware/tests/__init__.py | 12 +- .../tests/test_submitting_problems.py | 6 +- .../courseware/tests/test_video_mongo.py | 2 +- .../courseware/tests/test_videoalpha_mongo.py | 2 +- lms/djangoapps/courseware/tests/tests.py | 9 +- .../instructor/tests/test_gradebook.py | 10 +- .../instructor_task/tests/test_base.py | 4 +- .../instructor_task/tests/test_integration.py | 4 +- lms/djangoapps/open_ended_grading/tests.py | 2 +- rakelib/django.rake | 5 - 109 files changed, 807 insertions(+), 789 deletions(-) delete mode 100644 cms/djangoapps/contentstore/management/commands/update_templates.py delete mode 100644 common/lib/xmodule/xmodule/templates/about/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/annotatable/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/course/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/default/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/discussion/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/html/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/html/everything.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/peer_grading/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/problem/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/problem/emptyadvanced.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/sequence/with_video.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/statictab/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/video/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/videoalpha/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/word_cloud/default.yaml diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4d117a9c73..04c8a5baae 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -43,6 +43,13 @@ history of background tasks for a given problem and student. Blades: Small UX fix on capa multiple-choice problems. Make labels only as wide as the text to reduce accidental choice selections. +Studio: +- use xblock field defaults to initialize all new instances' fields and +only use templates as override samples. +- create new instances via in memory create_xmodule and related methods rather +than cloning a db record. +- have an explicit method for making a draft copy as distinct from making a new module. + Studio: Remove XML from the video component editor. All settings are moved to be edited as metadata. diff --git a/README.md b/README.md index e533459c8b..2208fe1cad 100644 --- a/README.md +++ b/README.md @@ -239,7 +239,6 @@ CMS templates. Fortunately, `rake` will do all of this for you! Just run: $ rake django-admin[syncdb] $ rake django-admin[migrate] - $ rake cms:update_templates If you are running these commands using the [`zsh`](http://www.zsh.org/) shell, zsh will assume that you are doing diff --git a/cms/djangoapps/contentstore/course_info_model.py b/cms/djangoapps/contentstore/course_info_model.py index ada3873992..7e1e6470ff 100644 --- a/cms/djangoapps/contentstore/course_info_model.py +++ b/cms/djangoapps/contentstore/course_info_model.py @@ -20,8 +20,8 @@ def get_course_updates(location): try: course_updates = modulestore('direct').get_item(location) except ItemNotFoundError: - template = Location(['i4x', 'edx', "templates", 'course_info', "Empty"]) - course_updates = modulestore('direct').clone_item(template, Location(location)) + modulestore('direct').create_and_save_xmodule(location) + course_updates = modulestore('direct').get_item(location) # current db rep: {"_id" : locationjson, "definition" : { "data" : "
          [

        1. date

          content
          2. ]
        "} "metadata" : ignored} location_base = course_updates.location.url() diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index cb24af47e0..756adad7c4 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -208,7 +208,7 @@ def set_date_and_time(date_css, desired_date, time_css, desired_time): def i_created_a_video_component(step): world.create_component_instance( step, '.large-video-icon', - 'i4x://edx/templates/video/default', + 'video', '.xmodule_VideoModule' ) diff --git a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py index 43164f62be..2f1788c6a4 100644 --- a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py +++ b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py @@ -7,9 +7,9 @@ from terrain.steps import reload_the_page @world.absorb -def create_component_instance(step, component_button_css, instance_id, expected_css): +def create_component_instance(step, component_button_css, category, expected_css, boilerplate=None): click_new_component_button(step, component_button_css) - click_component_from_menu(instance_id, expected_css) + click_component_from_menu(category, boilerplate, expected_css) @world.absorb @@ -19,7 +19,7 @@ def click_new_component_button(step, component_button_css): @world.absorb -def click_component_from_menu(instance_id, expected_css): +def click_component_from_menu(category, boilerplate, expected_css): """ Creates a component from `instance_id`. For components with more than one template, clicks on `elem_css` to create the new @@ -27,11 +27,13 @@ def click_component_from_menu(instance_id, expected_css): as the user clicks the appropriate button, so we assert that the expected component is present. """ - elem_css = "a[data-location='%s']" % instance_id + if boilerplate: + elem_css = "a[data-category='{}'][data-boilerplate='{}']".format(category, boilerplate) + else: + elem_css = "a[data-category='{}']:not([data-boilerplate])".format(category) elements = world.css_find(elem_css) - assert(len(elements) == 1) - if elements[0]['id'] == instance_id: # If this is a component with multiple templates - world.css_click(elem_css) + assert_equal(len(elements), 1) + world.css_click(elem_css) assert_equal(1, len(world.css_find(expected_css))) diff --git a/cms/djangoapps/contentstore/features/discussion-editor.py b/cms/djangoapps/contentstore/features/discussion-editor.py index a4a4b71668..8e4becb62e 100644 --- a/cms/djangoapps/contentstore/features/discussion-editor.py +++ b/cms/djangoapps/contentstore/features/discussion-editor.py @@ -8,7 +8,7 @@ from lettuce import world, step def i_created_discussion_tag(step): world.create_component_instance( step, '.large-discussion-icon', - 'i4x://edx/templates/discussion/Discussion_Tag', + 'discussion', '.xmodule_DiscussionModule' ) @@ -26,5 +26,5 @@ def i_see_only_the_settings_and_values(step): @step('creating a discussion takes a single click') def discussion_takes_a_single_click(step): assert(not world.is_css_present('.xmodule_DiscussionModule')) - world.css_click("a[data-location='i4x://edx/templates/discussion/Discussion_Tag']") + world.css_click("a[data-category='discussion']") assert(world.is_css_present('.xmodule_DiscussionModule')) diff --git a/cms/djangoapps/contentstore/features/html-editor.py b/cms/djangoapps/contentstore/features/html-editor.py index 53462ba094..b03388c89a 100644 --- a/cms/djangoapps/contentstore/features/html-editor.py +++ b/cms/djangoapps/contentstore/features/html-editor.py @@ -7,7 +7,7 @@ from lettuce import world, step @step('I have created a Blank HTML Page$') def i_created_blank_html_page(step): world.create_component_instance( - step, '.large-html-icon', 'i4x://edx/templates/html/Blank_HTML_Page', + step, '.large-html-icon', 'html', '.xmodule_HtmlModule' ) diff --git a/cms/djangoapps/contentstore/features/problem-editor.py b/cms/djangoapps/contentstore/features/problem-editor.py index 15f5da95e9..99b693225d 100644 --- a/cms/djangoapps/contentstore/features/problem-editor.py +++ b/cms/djangoapps/contentstore/features/problem-editor.py @@ -158,7 +158,7 @@ def create_latex_problem(step): world.click_new_component_button(step, '.large-problem-icon') # Go to advanced tab. world.css_click('#ui-id-2') - world.click_component_from_menu("i4x://edx/templates/problem/Problem_Written_in_LaTeX", '.xmodule_CapaModule') + world.click_component_from_menu("problem", "latex_problem.yaml", '.xmodule_CapaModule') @step('I edit and compile the High Level Source') diff --git a/cms/djangoapps/contentstore/features/studio-overview-togglesection.py b/cms/djangoapps/contentstore/features/studio-overview-togglesection.py index 9ab17fbdac..41e39513ea 100644 --- a/cms/djangoapps/contentstore/features/studio-overview-togglesection.py +++ b/cms/djangoapps/contentstore/features/studio-overview-togglesection.py @@ -22,7 +22,7 @@ def have_a_course_with_1_section(step): section = world.ItemFactory.create(parent_location=course.location) subsection1 = world.ItemFactory.create( parent_location=section.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name='Subsection One',) @@ -33,18 +33,18 @@ def have_a_course_with_two_sections(step): section = world.ItemFactory.create(parent_location=course.location) subsection1 = world.ItemFactory.create( parent_location=section.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name='Subsection One',) section2 = world.ItemFactory.create( parent_location=course.location, display_name='Section Two',) subsection2 = world.ItemFactory.create( parent_location=section2.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name='Subsection Alpha',) subsection3 = world.ItemFactory.create( parent_location=section2.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name='Subsection Beta',) diff --git a/cms/djangoapps/contentstore/features/video.py b/cms/djangoapps/contentstore/features/video.py index cb59193f17..a6a362befc 100644 --- a/cms/djangoapps/contentstore/features/video.py +++ b/cms/djangoapps/contentstore/features/video.py @@ -14,7 +14,7 @@ def does_not_autoplay(_step): @step('creating a video takes a single click') def video_takes_a_single_click(_step): assert(not world.is_css_present('.xmodule_VideoModule')) - world.css_click("a[data-location='i4x://edx/templates/video/default']") + world.css_click("a[data-category='video']") assert(world.is_css_present('.xmodule_VideoModule')) diff --git a/cms/djangoapps/contentstore/management/commands/update_templates.py b/cms/djangoapps/contentstore/management/commands/update_templates.py deleted file mode 100644 index 36348314b9..0000000000 --- a/cms/djangoapps/contentstore/management/commands/update_templates.py +++ /dev/null @@ -1,10 +0,0 @@ -from xmodule.templates import update_templates -from xmodule.modulestore.django import modulestore -from django.core.management.base import BaseCommand - - -class Command(BaseCommand): - help = 'Imports and updates the Studio component templates from the code pack and put in the DB' - - def handle(self, *args, **options): - update_templates(modulestore('direct')) diff --git a/cms/djangoapps/contentstore/module_info_model.py b/cms/djangoapps/contentstore/module_info_model.py index 726d4bb0ce..bce4b0326c 100644 --- a/cms/djangoapps/contentstore/module_info_model.py +++ b/cms/djangoapps/contentstore/module_info_model.py @@ -3,13 +3,13 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore import Location -def get_module_info(store, location, parent_location=None, rewrite_static_links=False): +def get_module_info(store, location, rewrite_static_links=False): try: module = store.get_item(location) except ItemNotFoundError: # create a new one - template_location = Location(['i4x', 'edx', 'templates', location.category, 'Empty']) - module = store.clone_item(template_location, location) + store.create_and_save_xmodule(location) + module = store.get_item(location) data = module.data if rewrite_static_links: @@ -29,7 +29,8 @@ def get_module_info(store, location, parent_location=None, rewrite_static_links= 'id': module.location.url(), 'data': data, # TODO (cpennington): This really shouldn't have to do this much reaching in to get the metadata - 'metadata': module._model_data._kvs._metadata + # what's the intent here? all metadata incl inherited & namespaced? + 'metadata': module.xblock_kvs._metadata } @@ -37,14 +38,11 @@ def set_module_info(store, location, post_data): module = None try: module = store.get_item(location) - except: - pass - - if module is None: - # new module at this location - # presume that we have an 'Empty' template - template_location = Location(['i4x', 'edx', 'templates', location.category, 'Empty']) - module = store.clone_item(template_location, location) + except ItemNotFoundError: + # new module at this location: almost always used for the course about pages; thus, no parent. (there + # are quite a handful of about page types available for a course and only the overview is pre-created) + store.create_and_save_xmodule(location) + module = store.get_item(location) if post_data.get('data') is not None: data = post_data['data'] @@ -79,4 +77,4 @@ def set_module_info(store, location, post_data): # commit to datastore # TODO (cpennington): This really shouldn't have to do this much reaching in to get the metadata - store.update_metadata(location, module._model_data._kvs._metadata) + store.update_metadata(location, module.xblock_kvs._metadata) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 6099b60eb1..fa7c45cb1d 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -24,12 +24,11 @@ from auth.authz import add_user_to_creator_group from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from xmodule.modulestore import Location +from xmodule.modulestore import Location, mongo from xmodule.modulestore.store_utilities import clone_course from xmodule.modulestore.store_utilities import delete_course from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore, _CONTENTSTORE -from xmodule.templates import update_templates from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint from xmodule.modulestore.inheritance import own_metadata @@ -183,7 +182,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) - draft_store.clone_item(html_module.location, html_module.location) + draft_store.convert_to_draft(html_module.location) # now query get_items() to get this location with revision=None, this should just # return back a single item (not 2) @@ -215,7 +214,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertEqual(html_module.lms.graceperiod, course.lms.graceperiod) self.assertNotIn('graceperiod', own_metadata(html_module)) - draft_store.clone_item(html_module.location, html_module.location) + draft_store.convert_to_draft(html_module.location) # refetch to check metadata html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) @@ -233,7 +232,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertNotIn('graceperiod', own_metadata(html_module)) # put back in draft and change metadata and see if it's now marked as 'own_metadata' - draft_store.clone_item(html_module.location, html_module.location) + draft_store.convert_to_draft(html_module.location) html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) new_graceperiod = timedelta(hours=1) @@ -255,7 +254,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): draft_store.publish(html_module.location, 0) # and re-read and verify 'own-metadata' - draft_store.clone_item(html_module.location, html_module.location) + draft_store.convert_to_draft(html_module.location) html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) self.assertIn('graceperiod', own_metadata(html_module)) @@ -278,7 +277,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): ) # put into draft - modulestore('draft').clone_item(problem.location, problem.location) + modulestore('draft').convert_to_draft(problem.location) # make sure we can query that item and verify that it is a draft draft_problem = modulestore('draft').get_item( @@ -574,7 +573,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_clone_course(self): course_data = { - 'template': 'i4x://edx/templates/course/Empty', 'org': 'MITx', 'number': '999', 'display_name': 'Robot Super Course', @@ -614,10 +612,10 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') location = Location('i4x://MITx/999/chapter/neuvo') - self.assertRaises(InvalidVersionError, draft_store.clone_item, 'i4x://edx/templates/chapter/Empty', - location) - direct_store.clone_item('i4x://edx/templates/chapter/Empty', location) - self.assertRaises(InvalidVersionError, draft_store.clone_item, location, location) + # Ensure draft mongo store does not allow us to create chapters either directly or via convert to draft + self.assertRaises(InvalidVersionError, draft_store.create_and_save_xmodule, location) + direct_store.create_and_save_xmodule(location) + self.assertRaises(InvalidVersionError, draft_store.convert_to_draft, location) self.assertRaises(InvalidVersionError, draft_store.update_item, location, 'chapter data') @@ -687,26 +685,35 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): import_from_xml(module_store, 'common/test/data/', ['toy']) location = CourseDescriptor.id_to_location('edX/toy/2012_Fall') - # get a vertical (and components in it) to put into 'draft' - vertical = module_store.get_item(Location(['i4x', 'edX', 'toy', - 'vertical', 'vertical_test', None]), depth=1) - - draft_store.clone_item(vertical.location, vertical.location) - + # get a vertical (and components in it) to copy into an orphan sub dag + vertical = module_store.get_item( + Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None]), + depth=1 + ) # We had a bug where orphaned draft nodes caused export to fail. This is here to cover that case. - draft_store.clone_item(vertical.location, Location(['i4x', 'edX', 'toy', - 'vertical', 'no_references', 'draft'])) + vertical.location = mongo.draft.as_draft(vertical.location.replace(name='no_references')) + draft_store.save_xmodule(vertical) + orphan_vertical = draft_store.get_item(vertical.location) + self.assertEqual(orphan_vertical.location.name, 'no_references') + # get the original vertical (and components in it) to put into 'draft' + vertical = module_store.get_item( + Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None]), + depth=1) + self.assertEqual(len(orphan_vertical.children), len(vertical.children)) + draft_store.convert_to_draft(vertical.location) for child in vertical.get_children(): - draft_store.clone_item(child.location, child.location) + draft_store.convert_to_draft(child.location) root_dir = path(mkdtemp_clean()) - # now create a private vertical - private_vertical = draft_store.clone_item(vertical.location, - Location(['i4x', 'edX', 'toy', 'vertical', 'a_private_vertical', None])) + # now create a new/different private (draft only) vertical + vertical.location = mongo.draft.as_draft(Location(['i4x', 'edX', 'toy', 'vertical', 'a_private_vertical', None])) + draft_store.save_xmodule(vertical) + private_vertical = draft_store.get_item(vertical.location) + vertical = None # blank out b/c i destructively manipulated its location 2 lines above - # add private to list of children + # add the new private to list of children sequential = module_store.get_item(Location(['i4x', 'edX', 'toy', 'sequential', 'vertical_sequential', None])) private_location_no_draft = private_vertical.location.replace(revision=None) @@ -885,7 +892,6 @@ class ContentStoreTest(ModuleStoreTestCase): self.client.login(username=uname, password=password) self.course_data = { - 'template': 'i4x://edx/templates/course/Empty', 'org': 'MITx', 'number': '999', 'display_name': 'Robot Super Course', @@ -1029,17 +1035,17 @@ class ContentStoreTest(ModuleStoreTestCase): html=True ) - def test_clone_item(self): + def test_create_item(self): """Test cloning an item. E.g. creating a new section""" CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') section_data = { 'parent_location': 'i4x://MITx/999/course/Robot_Super_Course', - 'template': 'i4x://edx/templates/chapter/Empty', + 'category': 'chapter', 'display_name': 'Section One', } - resp = self.client.post(reverse('clone_item'), section_data) + resp = self.client.post(reverse('create_item'), section_data) self.assertEqual(resp.status_code, 200) data = parse_json(resp) @@ -1054,14 +1060,14 @@ class ContentStoreTest(ModuleStoreTestCase): problem_data = { 'parent_location': 'i4x://MITx/999/course/Robot_Super_Course', - 'template': 'i4x://edx/templates/problem/Blank_Common_Problem' + 'category': 'problem' } - resp = self.client.post(reverse('clone_item'), problem_data) + resp = self.client.post(reverse('create_item'), problem_data) self.assertEqual(resp.status_code, 200) payload = parse_json(resp) - problem_loc = payload['id'] + problem_loc = Location(payload['id']) problem = get_modulestore(problem_loc).get_item(problem_loc) # should be a CapaDescriptor self.assertIsInstance(problem, CapaDescriptor, "New problem is not a CapaDescriptor") @@ -1194,10 +1200,9 @@ class ContentStoreTest(ModuleStoreTestCase): CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') new_component_location = Location('i4x', 'edX', '999', 'discussion', 'new_component') - source_template_location = Location('i4x', 'edx', 'templates', 'discussion', 'Discussion_Tag') # crate a new module and add it as a child to a vertical - module_store.clone_item(source_template_location, new_component_location) + module_store.create_and_save_xmodule(new_component_location) new_discussion_item = module_store.get_item(new_component_location) @@ -1218,10 +1223,9 @@ class ContentStoreTest(ModuleStoreTestCase): module_store.modulestore_update_signal.connect(_signal_hander) new_component_location = Location('i4x', 'edX', '999', 'html', 'new_component') - source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') # crate a new module - module_store.clone_item(source_template_location, new_component_location) + module_store.create_and_save_xmodule(new_component_location) finally: module_store.modulestore_update_signal = None @@ -1239,14 +1243,14 @@ class ContentStoreTest(ModuleStoreTestCase): # let's assert on the metadata_inheritance on an existing vertical for vertical in verticals: self.assertEqual(course.lms.xqa_key, vertical.lms.xqa_key) + self.assertEqual(course.start, vertical.lms.start) self.assertGreater(len(verticals), 0) new_component_location = Location('i4x', 'edX', 'toy', 'html', 'new_component') - source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') # crate a new module and add it as a child to a vertical - module_store.clone_item(source_template_location, new_component_location) + module_store.create_and_save_xmodule(new_component_location) parent = verticals[0] module_store.update_children(parent.location, parent.children + [new_component_location.url()]) @@ -1256,6 +1260,8 @@ class ContentStoreTest(ModuleStoreTestCase): # check for grace period definition which should be defined at the course level self.assertEqual(parent.lms.graceperiod, new_module.lms.graceperiod) + self.assertEqual(parent.lms.start, new_module.lms.start) + self.assertEqual(course.start, new_module.lms.start) self.assertEqual(course.lms.xqa_key, new_module.lms.xqa_key) @@ -1293,29 +1299,3 @@ class ContentStoreTest(ModuleStoreTestCase): self.assertEqual(course.textbooks, fetched_course.textbooks) # is this test too strict? i.e., it requires the dicts to be == self.assertEqual(course.checklists, fetched_course.checklists) - -class TemplateTestCase(ModuleStoreTestCase): - - def test_template_cleanup(self): - module_store = modulestore('direct') - - # insert a bogus template in the store - bogus_template_location = Location('i4x', 'edx', 'templates', 'html', 'bogus') - source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') - - module_store.clone_item(source_template_location, bogus_template_location) - - verify_create = module_store.get_item(bogus_template_location) - self.assertIsNotNone(verify_create) - - # now run cleanup - update_templates(modulestore('direct')) - - # now try to find dangling template, it should not be in DB any longer - asserted = False - try: - verify_create = module_store.get_item(bogus_template_location) - except ItemNotFoundError: - asserted = True - - self.assertTrue(asserted) diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 6c23e68240..fc04ad0a58 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -19,6 +19,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from models.settings.course_metadata import CourseMetadata from xmodule.modulestore.xml_importer import import_from_xml +from xmodule.modulestore.django import modulestore from xmodule.fields import Date from .utils import CourseTestCase diff --git a/cms/djangoapps/contentstore/tests/test_i18n.py b/cms/djangoapps/contentstore/tests/test_i18n.py index a292b7316e..88df19ec2d 100644 --- a/cms/djangoapps/contentstore/tests/test_i18n.py +++ b/cms/djangoapps/contentstore/tests/test_i18n.py @@ -35,7 +35,6 @@ class InternationalizationTest(ModuleStoreTestCase): self.user.save() self.course_data = { - 'template': 'i4x://edx/templates/course/Empty', 'org': 'MITx', 'number': '999', 'display_name': 'Robot Super Course', diff --git a/cms/djangoapps/contentstore/tests/test_item.py b/cms/djangoapps/contentstore/tests/test_item.py index 4e6c951d9b..2b514c0726 100644 --- a/cms/djangoapps/contentstore/tests/test_item.py +++ b/cms/djangoapps/contentstore/tests/test_item.py @@ -1,6 +1,9 @@ from contentstore.tests.test_course_settings import CourseTestCase from xmodule.modulestore.tests.factories import CourseFactory from django.core.urlresolvers import reverse +from xmodule.capa_module import CapaDescriptor +import json +from xmodule.modulestore.django import modulestore class DeleteItem(CourseTestCase): @@ -11,14 +14,199 @@ class DeleteItem(CourseTestCase): def testDeleteStaticPage(self): # Add static tab - data = { + data = json.dumps({ 'parent_location': 'i4x://mitX/333/course/Dummy_Course', - 'template': 'i4x://edx/templates/static_tab/Empty' - } + 'category': 'static_tab' + }) - resp = self.client.post(reverse('clone_item'), data) + resp = self.client.post(reverse('create_item'), data, + content_type="application/json") self.assertEqual(resp.status_code, 200) # Now delete it. There was a bug that the delete was failing (static tabs do not exist in draft modulestore). resp = self.client.post(reverse('delete_item'), resp.content, "application/json") self.assertEqual(resp.status_code, 200) + + +class TestCreateItem(CourseTestCase): + """ + Test the create_item handler thoroughly + """ + def response_id(self, response): + """ + Get the id from the response payload + :param response: + """ + parsed = json.loads(response.content) + return parsed['id'] + + def test_create_nicely(self): + """ + Try the straightforward use cases + """ + # create a chapter + display_name = 'Nicely created' + resp = self.client.post( + reverse('create_item'), + json.dumps({ + 'parent_location': self.course_location.url(), + 'display_name': display_name, + 'category': 'chapter' + }), + content_type="application/json" + ) + self.assertEqual(resp.status_code, 200) + + # get the new item and check its category and display_name + chap_location = self.response_id(resp) + new_obj = modulestore().get_item(chap_location) + self.assertEqual(new_obj.category, 'chapter') + self.assertEqual(new_obj.display_name, display_name) + self.assertEqual(new_obj.location.org, self.course_location.org) + self.assertEqual(new_obj.location.course, self.course_location.course) + + # get the course and ensure it now points to this one + course = modulestore().get_item(self.course_location) + self.assertIn(chap_location, course.children) + + # use default display name + resp = self.client.post( + reverse('create_item'), + json.dumps({ + 'parent_location': chap_location, + 'category': 'vertical' + }), + content_type="application/json" + ) + self.assertEqual(resp.status_code, 200) + + vert_location = self.response_id(resp) + + # create problem w/ boilerplate + template_id = 'multiplechoice.yaml' + resp = self.client.post( + reverse('create_item'), + json.dumps({ + 'parent_location': vert_location, + 'category': 'problem', + 'boilerplate': template_id + }), + content_type="application/json" + ) + self.assertEqual(resp.status_code, 200) + prob_location = self.response_id(resp) + problem = modulestore('draft').get_item(prob_location) + # ensure it's draft + self.assertTrue(problem.is_draft) + # check against the template + template = CapaDescriptor.get_template(template_id) + self.assertEqual(problem.data, template['data']) + self.assertEqual(problem.display_name, template['metadata']['display_name']) + self.assertEqual(problem.markdown, template['metadata']['markdown']) + + def test_create_item_negative(self): + """ + Negative tests for create_item + """ + # non-existent boilerplate: creates a default + resp = self.client.post( + reverse('create_item'), + json.dumps( + {'parent_location': self.course_location.url(), + 'category': 'problem', + 'boilerplate': 'nosuchboilerplate.yaml' + }), + content_type="application/json" + ) + self.assertEqual(resp.status_code, 200) + +class TestEditItem(CourseTestCase): + """ + Test contentstore.views.item.save_item + """ + def response_id(self, response): + """ + Get the id from the response payload + :param response: + """ + parsed = json.loads(response.content) + return parsed['id'] + + def setUp(self): + """ Creates the test course structure and a couple problems to 'edit'. """ + super(TestEditItem, self).setUp() + # create a chapter + display_name = 'chapter created' + resp = self.client.post( + reverse('create_item'), + json.dumps( + {'parent_location': self.course_location.url(), + 'display_name': display_name, + 'category': 'chapter' + }), + content_type="application/json" + ) + chap_location = self.response_id(resp) + resp = self.client.post( + reverse('create_item'), + json.dumps( + {'parent_location': chap_location, + 'category': 'vertical' + }), + content_type="application/json" + ) + vert_location = self.response_id(resp) + # create problem w/ boilerplate + template_id = 'multiplechoice.yaml' + resp = self.client.post( + reverse('create_item'), + json.dumps({'parent_location': vert_location, + 'category': 'problem', + 'boilerplate': template_id + }), + content_type="application/json" + ) + self.problems = [self.response_id(resp)] + + def test_delete_field(self): + """ + Sending null in for a field 'deletes' it + """ + self.client.post( + reverse('save_item'), + json.dumps({ + 'id': self.problems[0], + 'metadata': {'rerandomize': 'onreset'} + }), + content_type="application/json" + ) + problem = modulestore('draft').get_item(self.problems[0]) + self.assertEqual(problem.rerandomize, 'onreset') + self.client.post( + reverse('save_item'), + json.dumps({ + 'id': self.problems[0], + 'metadata': {'rerandomize': None} + }), + content_type="application/json" + ) + problem = modulestore('draft').get_item(self.problems[0]) + self.assertEqual(problem.rerandomize, 'never') + + + def test_null_field(self): + """ + Sending null in for a field 'deletes' it + """ + problem = modulestore('draft').get_item(self.problems[0]) + self.assertIsNotNone(problem.markdown) + self.client.post( + reverse('save_item'), + json.dumps({ + 'id': self.problems[0], + 'nullout': ['markdown'] + }), + content_type="application/json" + ) + problem = modulestore('draft').get_item(self.problems[0]) + self.assertIsNo/ne(problem.markdown) diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index bc9e9e8bae..a3f211a703 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -54,7 +54,6 @@ class CourseTestCase(ModuleStoreTestCase): self.client.login(username=uname, password=password) self.course = CourseFactory.create( - template='i4x://edx/templates/course/Empty', org='MITx', number='999', display_name='Robot Super Course', diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 5fa0d949b0..4973bddaca 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -19,14 +19,14 @@ NOTES_PANEL = {"name": _("My Notes"), "type": "notes"} EXTRA_TAB_PANELS = dict([(p['type'], p) for p in [OPEN_ENDED_PANEL, NOTES_PANEL]]) -def get_modulestore(location): +def get_modulestore(category_or_location): """ Returns the correct modulestore to use for modifying the specified location """ - if not isinstance(location, Location): - location = Location(location) + if isinstance(category_or_location, Location): + category_or_location = category_or_location.category - if location.category in DIRECT_ONLY_CATEGORIES: + if category_or_location in DIRECT_ONLY_CATEGORIES: return modulestore('direct') else: return modulestore() diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py index fa0a7b7b62..fdb5857ba7 100644 --- a/cms/djangoapps/contentstore/views/checklist.py +++ b/cms/djangoapps/contentstore/views/checklist.py @@ -7,11 +7,11 @@ from django.views.decorators.http import require_http_methods from django_future.csrf import ensure_csrf_cookie from mitxmako.shortcuts import render_to_response -from xmodule.modulestore import Location from xmodule.modulestore.inheritance import own_metadata from ..utils import get_modulestore, get_url_reverse from .access import get_location_and_verify_access +from xmodule.course_module import CourseDescriptor __all__ = ['get_checklists', 'update_checklist'] @@ -28,13 +28,11 @@ def get_checklists(request, org, course, name): modulestore = get_modulestore(location) course_module = modulestore.get_item(location) - new_course_template = Location('i4x', 'edx', 'templates', 'course', 'Empty') - template_module = modulestore.get_item(new_course_template) # If course was created before checklists were introduced, copy them over from the template. copied = False if not course_module.checklists: - course_module.checklists = template_module.checklists + course_module.checklists = CourseDescriptor.checklists.default copied = True checklists, modified = expand_checklist_action_urls(course_module) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 30958d5866..13eca522dd 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -26,6 +26,8 @@ from models.settings.course_grading import CourseGradingModel from .requests import _xmodule_recurse from .access import has_access +from xmodule.x_module import XModuleDescriptor +from xblock.plugin import PluginMissingError __all__ = ['OPEN_ENDED_COMPONENT_TYPES', 'ADVANCED_COMPONENT_POLICY_KEY', @@ -101,7 +103,7 @@ def edit_subsection(request, location): return render_to_response('edit_subsection.html', {'subsection': item, 'context_course': course, - 'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty'), + 'new_unit_category': 'vertical', 'lms_link': lms_link, 'preview_link': preview_link, 'course_graders': json.dumps(CourseGradingModel.fetch(course.location).graders), @@ -134,10 +136,27 @@ def edit_unit(request, location): item = modulestore().get_item(location, depth=1) except ItemNotFoundError: return HttpResponseBadRequest() - lms_link = get_lms_link_for_item(item.location, course_id=course.location.course_id) component_templates = defaultdict(list) + for category in COMPONENT_TYPES: + component_class = XModuleDescriptor.load_class(category) + # add the default template + has_markdown = hasattr(component_class, 'markdown') and component_class.markdown is not None + component_templates[category].append(( + component_class.display_name.default or 'Blank', + category, + has_markdown, + None # no boilerplate for overrides + )) + # add boilerplates + for template in component_class.templates(): + component_templates[category].append(( + template['metadata'].get('display_name'), + category, + template['metadata'].get('markdown') is not None, + template.get('template_id') + )) # Check if there are any advanced modules specified in the course policy. These modules # should be specified as a list of strings, where the strings are the names of the modules @@ -145,29 +164,29 @@ def edit_unit(request, location): course_advanced_keys = course.advanced_modules # Set component types according to course policy file - component_types = list(COMPONENT_TYPES) if isinstance(course_advanced_keys, list): - course_advanced_keys = [c for c in course_advanced_keys if c in ADVANCED_COMPONENT_TYPES] - if len(course_advanced_keys) > 0: - component_types.append(ADVANCED_COMPONENT_CATEGORY) + for category in course_advanced_keys: + if category in ADVANCED_COMPONENT_TYPES: + # Do I need to allow for boilerplates or just defaults on the class? i.e., can an advanced + # have more than one entry in the menu? one for default and others for prefilled boilerplates? + try: + component_class = XModuleDescriptor.load_class(category) + + component_templates['advanced'].append(( + component_class.display_name.default or category, + category, + hasattr(component_class, 'markdown') and component_class.markdown is not None, + None # don't override default data + )) + except PluginMissingError: + # dhm: I got this once but it can happen any time the course author configures + # an advanced component which does not exist on the server. This code here merely + # prevents any authors from trying to instantiate the non-existent component type + # by not showing it in the menu + pass else: log.error("Improper format for course advanced keys! {0}".format(course_advanced_keys)) - templates = modulestore().get_items(Location('i4x', 'edx', 'templates')) - for template in templates: - category = template.location.category - - if category in course_advanced_keys: - category = ADVANCED_COMPONENT_CATEGORY - - if category in component_types: - # This is a hack to create categories for different xmodules - component_templates[category].append(( - template.display_name_with_default, - template.location.url(), - hasattr(template, 'markdown') and template.markdown is not None - )) - components = [ component.location.url() for component @@ -219,7 +238,7 @@ def edit_unit(request, location): 'subsection': containing_subsection, 'release_date': get_default_time_display(containing_subsection.lms.start) if containing_subsection.lms.start is not None else None, 'section': containing_section, - 'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty'), + 'new_unit_category': 'vertical', 'unit_state': unit_state, 'published_date': get_default_time_display(item.cms.published_date) if item.cms.published_date is not None else None }) @@ -253,7 +272,7 @@ def create_draft(request): # This clones the existing item location to a draft location (the draft is implicit, # because modulestore is a Draft modulestore) - modulestore().clone_item(location, location) + modulestore().convert_to_draft(location) return HttpResponse() diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index f8de053d95..4c95d6e06e 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -82,10 +82,11 @@ def course_index(request, org, course, name): 'sections': sections, 'course_graders': json.dumps(CourseGradingModel.fetch(course.location).graders), 'parent_location': course.location, - 'new_section_template': Location('i4x', 'edx', 'templates', 'chapter', 'Empty'), - 'new_subsection_template': Location('i4x', 'edx', 'templates', 'sequential', 'Empty'), # for now they are the same, but the could be different at some point... + 'new_section_category': 'chapter', + 'new_subsection_category': 'sequential', 'upload_asset_callback_url': upload_asset_callback_url, - 'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty') + 'new_unit_category': 'vertical', + 'category': 'vertical' }) @@ -98,12 +99,6 @@ def create_new_course(request): if not is_user_in_creator_group(request.user): raise PermissionDenied() - # This logic is repeated in xmodule/modulestore/tests/factories.py - # so if you change anything here, you need to also change it there. - # TODO: write a test that creates two courses, one with the factory and - # the other with this method, then compare them to make sure they are - # equivalent. - template = Location(request.POST['template']) org = request.POST.get('org') number = request.POST.get('number') display_name = request.POST.get('display_name') @@ -121,29 +116,26 @@ def create_new_course(request): existing_course = modulestore('direct').get_item(dest_location) except ItemNotFoundError: pass - if existing_course is not None: return JsonResponse({'ErrMsg': 'There is already a course defined with this name.'}) course_search_location = ['i4x', dest_location.org, dest_location.course, 'course', None] courses = modulestore().get_items(course_search_location) - if len(courses) > 0: return JsonResponse({'ErrMsg': 'There is already a course defined with the same organization and course number.'}) - new_course = modulestore('direct').clone_item(template, dest_location) + # instantiate the CourseDescriptor and then persist it + # note: no system to pass + if display_name is None: + metadata = {} + else: + metadata = {'display_name': display_name} + modulestore('direct').create_and_save_xmodule(dest_location, metadata=metadata) + new_course = modulestore('direct').get_item(dest_location) # clone a default 'about' module as well - - about_template_location = Location(['i4x', 'edx', 'templates', 'about', 'overview']) dest_about_location = dest_location._replace(category='about', name='overview') - modulestore('direct').clone_item(about_template_location, dest_about_location) - - if display_name is not None: - new_course.display_name = display_name - - # set a default start date to now - new_course.start = datetime.datetime.now(UTC()) + modulestore('direct').create_and_save_xmodule(dest_about_location, system=new_course.system) initialize_course_tabs(new_course) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index abc5f48564..90dae10c23 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -13,16 +13,26 @@ from util.json_request import expect_json from ..utils import get_modulestore from .access import has_access from .requests import _xmodule_recurse +from xmodule.x_module import XModuleDescriptor -__all__ = ['save_item', 'clone_item', 'delete_item'] +__all__ = ['save_item', 'create_item', 'delete_item'] # cdodge: these are categories which should not be parented, they are detached from the hierarchy DETACHED_CATEGORIES = ['about', 'static_tab', 'course_info'] - @login_required @expect_json def save_item(request): + """ + Will carry a json payload with these possible fields + :id (required): the id + :data (optional): the new value for the data + :metadata (optional): new values for the metadata fields. + Any whose values are None will be deleted not set to None! Absent ones will be left alone + :nullout (optional): which metadata fields to set to None + """ + # The nullout is a bit of a temporary copout until we can make module_edit.coffee and the metadata editors a + # little smarter and able to pass something more akin to {unset: [field, field]} item_location = request.POST['id'] # check permissions for this user within this course @@ -42,30 +52,25 @@ def save_item(request): children = request.POST['children'] store.update_children(item_location, children) - # cdodge: also commit any metadata which might have been passed along in the - # POST from the client, if it is there - # NOTE, that the postback is not the complete metadata, as there's system metadata which is - # not presented to the end-user for editing. So let's fetch the original and - # 'apply' the submitted metadata, so we don't end up deleting system metadata - if request.POST.get('metadata') is not None: - posted_metadata = request.POST['metadata'] - # fetch original + # cdodge: also commit any metadata which might have been passed along + if request.POST.get('nullout') is not None or request.POST.get('metadata') is not None: + # the postback is not the complete metadata, as there's system metadata which is + # not presented to the end-user for editing. So let's fetch the original and + # 'apply' the submitted metadata, so we don't end up deleting system metadata existing_item = modulestore().get_item(item_location) + for metadata_key in request.POST.get('nullout', []): + setattr(existing_item, metadata_key, None) # update existing metadata with submitted metadata (which can be partial) - # IMPORTANT NOTE: if the client passed pack 'null' (None) for a piece of metadata that means 'remove it' - for metadata_key, value in posted_metadata.items(): + # IMPORTANT NOTE: if the client passed 'null' (None) for a piece of metadata that means 'remove it'. If + # the intent is to make it None, use the nullout field + for metadata_key, value in request.POST.get('metadata', {}).items(): - if posted_metadata[metadata_key] is None: - # remove both from passed in collection as well as the collection read in from the modulestore - if metadata_key in existing_item._model_data: - del existing_item._model_data[metadata_key] - del posted_metadata[metadata_key] + if value is None: + delattr(existing_item, metadata_key) else: - existing_item._model_data[metadata_key] = value - + setattr(existing_item, metadata_key, value) # commit to datastore - # TODO (cpennington): This really shouldn't have to do this much reaching in to get the metadata store.update_metadata(item_location, own_metadata(existing_item)) return HttpResponse() @@ -73,28 +78,38 @@ def save_item(request): @login_required @expect_json -def clone_item(request): +def create_item(request): parent_location = Location(request.POST['parent_location']) - template = Location(request.POST['template']) + category = request.POST['category'] display_name = request.POST.get('display_name') if not has_access(request.user, parent_location): raise PermissionDenied() - parent = get_modulestore(template).get_item(parent_location) - dest_location = parent_location._replace(category=template.category, name=uuid4().hex) + parent = get_modulestore(category).get_item(parent_location) + dest_location = parent_location.replace(category=category, name=uuid4().hex) - new_item = get_modulestore(template).clone_item(template, dest_location) + # get the metadata, display_name, and definition from the request + metadata = {} + data = None + template_id = request.POST.get('boilerplate') + if template_id is not None: + clz = XModuleDescriptor.load_class(category) + if clz is not None: + template = clz.get_template(template_id) + if template is not None: + metadata = template.get('metadata', {}) + data = template.get('data') - # replace the display name with an optional parameter passed in from the caller if display_name is not None: - new_item.display_name = display_name + metadata['display_name'] = display_name - get_modulestore(template).update_metadata(new_item.location.url(), own_metadata(new_item)) + get_modulestore(category).create_and_save_xmodule(dest_location, definition_data=data, + metadata=metadata, system=parent.system) - if new_item.location.category not in DETACHED_CATEGORIES: - get_modulestore(parent.location).update_children(parent_location, parent.children + [new_item.location.url()]) + if category not in DETACHED_CATEGORIES: + get_modulestore(parent.location).update_children(parent_location, parent.children + [dest_location.url()]) return HttpResponse(json.dumps({'id': dest_location.url()})) diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index 948ed614d2..ee6b0bf84d 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/cms/djangoapps/contentstore/views/user.py @@ -27,6 +27,7 @@ def index(request): # filter out courses that we don't have access too def course_filter(course): return (has_access(request.user, course.location) + # TODO remove this condition when templates purged from db and course.location.course != 'templates' and course.location.org != '' and course.location.course != '' @@ -34,7 +35,6 @@ def index(request): courses = filter(course_filter, courses) return render_to_response('index.html', { - 'new_course_template': Location('i4x', 'edx', 'templates', 'course', 'Empty'), 'courses': [(course.display_name, get_url_reverse('CourseOutline', course), get_lms_link_for_item(course.location, course_id=course.location.course_id)) diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py index 4ea9f2f5db..e529a284c6 100644 --- a/cms/djangoapps/models/settings/course_grading.py +++ b/cms/djangoapps/models/settings/course_grading.py @@ -9,7 +9,7 @@ class CourseGradingModel(object): """ def __init__(self, course_descriptor): self.course_location = course_descriptor.location - self.graders = [CourseGradingModel.jsonize_grader(i, grader) for i, grader in enumerate(course_descriptor.raw_grader)] # weights transformed to ints [0..100] + self.graders = [CourseGradingModel.jsonize_grader(i, grader) for i, grader in enumerate(course_descriptor.raw_grader)] # weights transformed to ints [0..100] self.grade_cutoffs = course_descriptor.grade_cutoffs self.grace_period = CourseGradingModel.convert_set_grace_period(course_descriptor) @@ -81,7 +81,7 @@ class CourseGradingModel(object): Decode the json into CourseGradingModel and save any changes. Returns the modified model. Probably not the usual path for updates as it's too coarse grained. """ - course_location = jsondict['course_location'] + course_location = Location(jsondict['course_location']) descriptor = get_modulestore(course_location).get_item(course_location) graders_parsed = [CourseGradingModel.parse_grader(jsonele) for jsonele in jsondict['graders']] @@ -89,7 +89,7 @@ class CourseGradingModel(object): descriptor.raw_grader = graders_parsed descriptor.grade_cutoffs = jsondict['grade_cutoffs'] - get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data) + get_modulestore(course_location).update_item(course_location, descriptor.xblock_kvs._data) CourseGradingModel.update_grace_period_from_json(course_location, jsondict['grace_period']) return CourseGradingModel.fetch(course_location) @@ -209,7 +209,7 @@ class CourseGradingModel(object): descriptor = get_modulestore(location).get_item(location) return {"graderType": descriptor.lms.format if descriptor.lms.format is not None else 'Not Graded', "location": location, - "id": 99 # just an arbitrary value to + "id": 99 # just an arbitrary value to } @staticmethod @@ -232,7 +232,7 @@ class CourseGradingModel(object): # 5 hours 59 minutes 59 seconds => converted to iso format rawgrace = descriptor.lms.graceperiod if rawgrace: - hours_from_days = rawgrace.days*24 + hours_from_days = rawgrace.days * 24 seconds = rawgrace.seconds hours_from_seconds = int(seconds / 3600) hours = hours_from_days + hours_from_seconds diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index 5154591d6f..62083fa26d 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -56,14 +56,15 @@ class CMS.Views.ModuleEdit extends Backbone.View changedMetadata: -> return _.extend(@metadataEditor.getModifiedMetadataValues(), @customMetadata()) - cloneTemplate: (parent, template) -> - $.post("/clone_item", { - parent_location: parent - template: template - }, (data) => - @model.set(id: data.id) - @$el.data('id', data.id) - @render() + createItem: (parent, payload) -> + payload.parent_location = parent + $.post( + "/create_item" + payload + (data) => + @model.set(id: data.id) + @$el.data('id', data.id) + @render() ) render: -> diff --git a/cms/static/coffee/src/views/tabs.coffee b/cms/static/coffee/src/views/tabs.coffee index 1034fc988e..58f52f27a3 100644 --- a/cms/static/coffee/src/views/tabs.coffee +++ b/cms/static/coffee/src/views/tabs.coffee @@ -55,9 +55,9 @@ class CMS.Views.TabsEdit extends Backbone.View editor.$el.removeClass('new') , 500) - editor.cloneTemplate( + editor.createItem( @model.get('id'), - 'i4x://edx/templates/static_tab/Empty' + {category: 'static_tab'} ) analytics.track "Added Static Page", diff --git a/cms/static/coffee/src/views/unit.coffee b/cms/static/coffee/src/views/unit.coffee index 058bcf0ce1..774ef04f6d 100644 --- a/cms/static/coffee/src/views/unit.coffee +++ b/cms/static/coffee/src/views/unit.coffee @@ -89,9 +89,9 @@ class CMS.Views.UnitEdit extends Backbone.View @$newComponentItem.before(editor.$el) - editor.cloneTemplate( + editor.createItem( @$el.data('id'), - $(event.currentTarget).data('location') + $(event.currentTarget).data() ) analytics.track "Added a Component", diff --git a/cms/static/js/base.js b/cms/static/js/base.js index b53d116808..3d8cd7684e 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -338,7 +338,7 @@ function createNewUnit(e) { e.preventDefault(); var parent = $(this).data('parent'); - var template = $(this).data('template'); + var category = $(this).data('category'); analytics.track('Created a Unit', { 'course': course_location_analytics, @@ -346,9 +346,9 @@ function createNewUnit(e) { }); - $.post('/clone_item', { + $.post('/create_item', { 'parent_location': parent, - 'template': template, + 'category': category, 'display_name': 'New Unit' }, @@ -551,7 +551,7 @@ function saveNewSection(e) { var $saveButton = $(this).find('.new-section-name-save'); var parent = $saveButton.data('parent'); - var template = $saveButton.data('template'); + var category = $saveButton.data('category'); var display_name = $(this).find('.new-section-name').val(); analytics.track('Created a Section', { @@ -559,9 +559,9 @@ function saveNewSection(e) { 'display_name': display_name }); - $.post('/clone_item', { + $.post('/create_item', { 'parent_location': parent, - 'template': template, + 'category': category, 'display_name': display_name, }, @@ -595,7 +595,6 @@ function saveNewCourse(e) { e.preventDefault(); var $newCourse = $(this).closest('.new-course'); - var template = $(this).find('.new-course-save').data('template'); var org = $newCourse.find('.new-course-org').val(); var number = $newCourse.find('.new-course-number').val(); var display_name = $newCourse.find('.new-course-name').val(); @@ -612,7 +611,6 @@ function saveNewCourse(e) { }); $.post('/create_new_course', { - 'template': template, 'org': org, 'number': number, 'display_name': display_name @@ -646,7 +644,7 @@ function addNewSubsection(e) { var parent = $(this).parents("section.branch").data("id"); $saveButton.data('parent', parent); - $saveButton.data('template', $(this).data('template')); + $saveButton.data('category', $(this).data('category')); $newSubsection.find('.new-subsection-form').bind('submit', saveNewSubsection); $cancelButton.bind('click', cancelNewSubsection); @@ -659,7 +657,7 @@ function saveNewSubsection(e) { e.preventDefault(); var parent = $(this).find('.new-subsection-name-save').data('parent'); - var template = $(this).find('.new-subsection-name-save').data('template'); + var category = $(this).find('.new-subsection-name-save').data('category'); var display_name = $(this).find('.new-subsection-name-input').val(); analytics.track('Created a Subsection', { @@ -668,9 +666,9 @@ function saveNewSubsection(e) { }); - $.post('/clone_item', { + $.post('/create_item', { 'parent_location': parent, - 'template': template, + 'category': category, 'display_name': display_name }, diff --git a/cms/templates/index.html b/cms/templates/index.html index 57921641ce..f0baef4f09 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -25,8 +25,8 @@
        - - + +
        diff --git a/cms/templates/overview.html b/cms/templates/overview.html index ab7788c64a..56836b00ad 100644 --- a/cms/templates/overview.html +++ b/cms/templates/overview.html @@ -1,4 +1,4 @@ -<%! from django.utils.translation import ugettext as _ %> +/<%! from django.utils.translation import ugettext as _ %> <%inherit file="base.html" /> <%! import logging @@ -66,7 +66,8 @@

        - +

        @@ -83,8 +84,9 @@ Click here to set the section name
        - - + +
        @@ -181,7 +183,7 @@
        - + ${_("New Subsection")}
        diff --git a/cms/templates/unit.html b/cms/templates/unit.html index ce4f239ab1..300d631421 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -59,8 +59,8 @@ % if type == 'advanced' or len(templates) > 1: % else: - % for __, location, __ in templates: - + % for __, category, __, __ in templates: + % endfor % endif @@ -74,49 +74,60 @@ % if len(templates) > 1 or type == 'advanced':
        % if type == "problem": -
        - - % endif -
        -
        - % endif - ${_("Cancel")} -
        - % endif + % endif +
        +
          + % for name, category, has_markdown, boilerplate_name in sorted(templates): + % if has_markdown or type != "problem": + % if boilerplate_name is None: +
        • + + ${name} + +
          • + + % else: +
        • + + ${name} + +
          • + % endif + % endif + + %endfor +
        +
        + % if type == "problem": +
        +
          + % for name, category, has_markdown, boilerplate_name in sorted(templates): + % if not has_markdown: +
        • + + ${name} + +
          • + % endif + % endfor +
        +
        +
        + % endif + Cancel +
        + % endif % endfor
      diff --git a/cms/templates/widgets/units.html b/cms/templates/widgets/units.html index 5ac05e79eb..62c1fb62d7 100644 --- a/cms/templates/widgets/units.html +++ b/cms/templates/widgets/units.html @@ -34,7 +34,7 @@ This def will enumerate through a passed in subsection and list all of the units % endfor
    1. - + New Unit
      2. diff --git a/cms/urls.py b/cms/urls.py index 6db07dc2ed..711742d89f 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -17,7 +17,7 @@ urlpatterns = ('', # nopep8 url(r'^preview_component/(?P.*?)$', 'contentstore.views.preview_component', name='preview_component'), url(r'^save_item$', 'contentstore.views.save_item', name='save_item'), url(r'^delete_item$', 'contentstore.views.delete_item', name='delete_item'), - url(r'^clone_item$', 'contentstore.views.clone_item', name='clone_item'), + url(r'^create_item$', 'contentstore.views.create_item', name='create_item'), url(r'^create_draft$', 'contentstore.views.create_draft', name='create_draft'), url(r'^publish_draft$', 'contentstore.views.publish_draft', name='publish_draft'), url(r'^unpublish_unit$', 'contentstore.views.unpublish_unit', name='unpublish_unit'), diff --git a/common/djangoapps/terrain/course_helpers.py b/common/djangoapps/terrain/course_helpers.py index 27bf95099d..c62b1a1e79 100644 --- a/common/djangoapps/terrain/course_helpers.py +++ b/common/djangoapps/terrain/course_helpers.py @@ -12,7 +12,6 @@ from django.contrib.sessions.middleware import SessionMiddleware from student.models import CourseEnrollment from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore -from xmodule.templates import update_templates from urllib import quote_plus @@ -84,5 +83,4 @@ def clear_courses(): # from the bash shell to drop it: # $ mongo test_xmodule --eval "db.dropDatabase()" modulestore().collection.drop() - update_templates(modulestore('direct')) contentstore().fs_files.drop() diff --git a/common/djangoapps/tests.py b/common/djangoapps/tests.py index 8e78ee7f37..4a61a106d4 100644 --- a/common/djangoapps/tests.py +++ b/common/djangoapps/tests.py @@ -23,15 +23,15 @@ class TestXmoduleModfiers(ModuleStoreTestCase): number='313', display_name='histogram test') section = ItemFactory.create( parent_location=course.location, display_name='chapter hist', - template='i4x://edx/templates/chapter/Empty') + category='chapter') problem = ItemFactory.create( parent_location=section.location, display_name='problem hist 1', - template='i4x://edx/templates/problem/Blank_Common_Problem') + category='problem') problem.has_score = False # don't trip trying to retrieve db data late_problem = ItemFactory.create( parent_location=section.location, display_name='problem hist 2', - template='i4x://edx/templates/problem/Blank_Common_Problem') + category='problem') late_problem.lms.start = datetime.datetime.now(UTC) + datetime.timedelta(days=32) late_problem.has_score = False diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py index 2e61076e94..53f080eb3a 100644 --- a/common/lib/xmodule/xmodule/abtest_module.py +++ b/common/lib/xmodule/xmodule/abtest_module.py @@ -80,8 +80,6 @@ class ABTestModule(ABTestFields, XModule): class ABTestDescriptor(ABTestFields, RawDescriptor, XmlDescriptor): module_class = ABTestModule - template_dir_name = "abtest" - @classmethod def definition_from_xml(cls, xml_object, system): """ diff --git a/common/lib/xmodule/xmodule/annotatable_module.py b/common/lib/xmodule/xmodule/annotatable_module.py index 8b1bbc71d3..f80e3e488e 100644 --- a/common/lib/xmodule/xmodule/annotatable_module.py +++ b/common/lib/xmodule/xmodule/annotatable_module.py @@ -150,5 +150,4 @@ class AnnotatableModule(AnnotatableFields, XModule): class AnnotatableDescriptor(AnnotatableFields, RawDescriptor): module_class = AnnotatableModule - template_dir_name = "annotatable" mako_template = "widgets/raw-edit.html" diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 13f00bb77e..60103a2126 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -290,7 +290,6 @@ class CombinedOpenEndedDescriptor(CombinedOpenEndedFields, RawDescriptor): has_score = True always_recalculate_grades = True - template_dir_name = "combinedopenended" #Specify whether or not to pass in S3 interface needs_s3_interface = True diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index ceadee1c15..d4aac5b0ae 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -366,8 +366,6 @@ class CourseFields(object): class CourseDescriptor(CourseFields, SequenceDescriptor): module_class = SequenceModule - template_dir_name = 'course' - def __init__(self, *args, **kwargs): """ Expects the same arguments as XModuleDescriptor.__init__ diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index a37081c447..db1f998187 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -96,6 +96,7 @@ class ErrorDescriptor(ErrorFields, JSONEditingDescriptor): 'contents': contents, 'display_name': 'Error: ' + location.name, 'location': location, + 'category': 'error' } return cls( system, @@ -109,12 +110,12 @@ class ErrorDescriptor(ErrorFields, JSONEditingDescriptor): } @classmethod - def from_json(cls, json_data, system, error_msg='Error not available'): + def from_json(cls, json_data, system, location, error_msg='Error not available'): return cls._construct( system, - json.dumps(json_data, indent=4), + json.dumps(json_data, skipkeys=False, indent=4), error_msg, - location=Location(json_data['location']), + location=location ) @classmethod diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py index fdab14b58d..c4e9e2d35c 100644 --- a/common/lib/xmodule/xmodule/foldit_module.py +++ b/common/lib/xmodule/xmodule/foldit_module.py @@ -184,7 +184,6 @@ class FolditDescriptor(FolditFields, XmlDescriptor, EditingDescriptor): filename_extension = "xml" has_score = True - template_dir_name = "foldit" js = {'coffee': [resource_string(__name__, 'js/src/html/edit.coffee')]} js_module_name = "HTMLEditingDescriptor" diff --git a/common/lib/xmodule/xmodule/gst_module.py b/common/lib/xmodule/xmodule/gst_module.py index e101d90b4c..5a4930ff95 100644 --- a/common/lib/xmodule/xmodule/gst_module.py +++ b/common/lib/xmodule/xmodule/gst_module.py @@ -141,7 +141,6 @@ class GraphicalSliderToolModule(GraphicalSliderToolFields, XModule): class GraphicalSliderToolDescriptor(GraphicalSliderToolFields, MakoModuleDescriptor, XmlDescriptor): module_class = GraphicalSliderToolModule - template_dir_name = 'graphical_slider_tool' @classmethod def definition_from_xml(cls, xml_object, system): diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 9ff2e4d9a8..2c7c9e0b01 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -234,7 +234,7 @@ class StaticTabDescriptor(StaticTabFields, HtmlDescriptor): These pieces of course content are treated as HtmlModules but we need to overload where the templates are located in order to be able to create new ones """ - template_dir_name = "statictab" + template_dir_name = None module_class = StaticTabModule @@ -261,5 +261,5 @@ class CourseInfoDescriptor(CourseInfoFields, HtmlDescriptor): These pieces of course content are treated as HtmlModules but we need to overload where the templates are located in order to be able to create new ones """ - template_dir_name = "courseinfo" + template_dir_name = None module_class = CourseInfoModule diff --git a/common/lib/xmodule/xmodule/js/spec/combinedopenended/edit_spec.coffee b/common/lib/xmodule/xmodule/js/spec/combinedopenended/edit_spec.coffee index aa077da450..d859a59dda 100644 --- a/common/lib/xmodule/xmodule/js/spec/combinedopenended/edit_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/combinedopenended/edit_spec.coffee @@ -11,13 +11,13 @@ describe 'OpenEndedMarkdownEditingDescriptor', -> @descriptor = new OpenEndedMarkdownEditingDescriptor($('.combinedopenended-editor')) @descriptor.createXMLEditor('replace with markdown') saveResult = @descriptor.save() - expect(saveResult.metadata.markdown).toEqual(null) + expect(saveResult.nullout).toEqual(['markdown']) expect(saveResult.data).toEqual('replace with markdown') it 'saves xml from the xml editor', -> loadFixtures 'combinedopenended-without-markdown.html' @descriptor = new OpenEndedMarkdownEditingDescriptor($('.combinedopenended-editor')) saveResult = @descriptor.save() - expect(saveResult.metadata.markdown).toEqual(null) + expect(saveResult.nullout).toEqual(['markdown']) expect(saveResult.data).toEqual('xml only') describe 'insertPrompt', -> diff --git a/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.coffee b/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.coffee index 5161e658e7..1df9587037 100644 --- a/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.coffee @@ -11,13 +11,13 @@ describe 'MarkdownEditingDescriptor', -> @descriptor = new MarkdownEditingDescriptor($('.problem-editor')) @descriptor.createXMLEditor('replace with markdown') saveResult = @descriptor.save() - expect(saveResult.metadata.markdown).toEqual(null) + expect(saveResult.nullout).toEqual(['markdown']) expect(saveResult.data).toEqual('replace with markdown') it 'saves xml from the xml editor', -> loadFixtures 'problem-without-markdown.html' @descriptor = new MarkdownEditingDescriptor($('.problem-editor')) saveResult = @descriptor.save() - expect(saveResult.metadata.markdown).toEqual(null) + expect(saveResult.nullout).toEqual(['markdown']) expect(saveResult.data).toEqual('xml only') describe 'insertMultipleChoice', -> diff --git a/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee b/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee index 1b7f9bb4fb..4cdd571c65 100644 --- a/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee +++ b/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee @@ -153,8 +153,7 @@ Write a persuasive essay to a newspaper reflecting your vies on censorship in li else { data: @xml_editor.getValue() - metadata: - markdown: null + nullout: ['markdown'] } @insertRubric: (selectedText) -> diff --git a/common/lib/xmodule/xmodule/js/src/problem/edit.coffee b/common/lib/xmodule/xmodule/js/src/problem/edit.coffee index b723f230e9..bd2871eb61 100644 --- a/common/lib/xmodule/xmodule/js/src/problem/edit.coffee +++ b/common/lib/xmodule/xmodule/js/src/problem/edit.coffee @@ -123,9 +123,8 @@ class @MarkdownEditingDescriptor extends XModule.Descriptor } else { - data: @xml_editor.getValue() - metadata: - markdown: null + data: @xml_editor.getValue() + nullout: ['markdown'] } @insertMultipleChoice: (selectedText) -> diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 2fa12e2e90..eb721dfc99 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -310,14 +310,7 @@ class ModuleStore(object): """ raise NotImplementedError - def clone_item(self, source, location): - """ - Clone a new item that is a copy of the item at the location `source` - and writes it to `location` - """ - raise NotImplementedError - - def update_item(self, location, data): + def update_item(self, location, data, allow_not_found=False): """ Set the data in the item specified by the location to data diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index f56393d75e..0b1c601a2f 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -33,7 +33,7 @@ from xblock.runtime import DbModel, KeyValueStore, InvalidScopeError from xblock.core import Scope from xmodule.modulestore import ModuleStoreBase, Location, namedtuple_to_son -from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateItemError +from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata, INHERITABLE_METADATA, inherit_metadata log = logging.getLogger(__name__) @@ -62,11 +62,12 @@ class MongoKeyValueStore(KeyValueStore): A KeyValueStore that maps keyed data access to one of the 3 data areas known to the MongoModuleStore (data, children, and metadata) """ - def __init__(self, data, children, metadata, location): + def __init__(self, data, children, metadata, location, category): self._data = data self._children = children self._metadata = metadata self._location = location + self._category = category def get(self, key): if key.scope == Scope.children: @@ -78,6 +79,8 @@ class MongoKeyValueStore(KeyValueStore): elif key.scope == Scope.content: if key.field_name == 'location': return self._location + elif key.field_name == 'category': + return self._category elif key.field_name == 'data' and not isinstance(self._data, dict): return self._data else: @@ -93,6 +96,8 @@ class MongoKeyValueStore(KeyValueStore): elif key.scope == Scope.content: if key.field_name == 'location': self._location = value + elif key.field_name == 'category': + self._category = value elif key.field_name == 'data' and not isinstance(self._data, dict): self._data = value else: @@ -109,6 +114,8 @@ class MongoKeyValueStore(KeyValueStore): elif key.scope == Scope.content: if key.field_name == 'location': self._location = Location(None) + elif key.field_name == 'category': + self._category = None elif key.field_name == 'data' and not isinstance(self._data, dict): self._data = None else: @@ -123,7 +130,10 @@ class MongoKeyValueStore(KeyValueStore): return key.field_name in self._metadata elif key.scope == Scope.content: if key.field_name == 'location': + # WHY TRUE? if it's been deleted should it be False? return True + elif key.field_name == 'category': + return self._category is not None elif key.field_name == 'data' and not isinstance(self._data, dict): return True else: @@ -185,8 +195,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem): else: # load the module and apply the inherited metadata try: + category = json_data['location']['category'] class_ = XModuleDescriptor.load_class( - json_data['location']['category'], + category, self.default_class ) definition = json_data.get('definition', {}) @@ -201,9 +212,12 @@ class CachingDescriptorSystem(MakoDescriptorSystem): definition.get('children', []), metadata, location, + category ) model_data = DbModel(kvs, class_, None, MongoUsage(self.course_id, location)) + model_data['category'] = category + model_data['location'] = location module = class_(self, model_data) if self.cached_metadata is not None: # parent container pointers don't differentiate between draft and non-draft @@ -217,6 +231,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): return ErrorDescriptor.from_json( json_data, self, + json_data['location'], error_msg=exc_info_to_str(sys.exc_info()) ) @@ -582,51 +597,97 @@ class MongoModuleStore(ModuleStoreBase): modules = self._load_items(list(items), depth) return modules - def clone_item(self, source, location): + def create_xmodule(self, location, definition_data=None, metadata=None, system=None): """ - Clone a new item that is a copy of the item at the location `source` - and writes it to `location` + Create the new xmodule but don't save it. Returns the new module. + + :param location: a Location--must have a category + :param definition_data: can be empty. The initial definition_data for the kvs + :param metadata: can be empty, the initial metadata for the kvs + :param system: if you already have an xmodule from the course, the xmodule.system value """ - item = None - try: - source_item = self.collection.find_one(location_to_query(source)) - - # allow for some programmatically generated substitutions in metadata, e.g. Discussion_id's should be auto-generated - for key in source_item['metadata'].keys(): - if source_item['metadata'][key] == '$$GUID$$': - source_item['metadata'][key] = uuid4().hex - - source_item['_id'] = Location(location).dict() - self.collection.insert( - source_item, - # Must include this to avoid the django debug toolbar (which defines the deprecated "safe=False") - # from overriding our default value set in the init method. - safe=self.collection.safe + if not isinstance(location, Location): + location = Location(location) + # differs from split mongo in that I believe most of this logic should be above the persistence + # layer but added it here to enable quick conversion. I'll need to reconcile these. + if metadata is None: + metadata = {} + if system is None: + system = CachingDescriptorSystem( + self, + {}, + self.default_class, + None, + self.error_tracker, + self.render_template, + {} ) - item = self._load_items([source_item])[0] + xblock_class = XModuleDescriptor.load_class(location.category, self.default_class) + if definition_data is None: + if hasattr(xblock_class, 'data') and getattr(xblock_class, 'data').default is not None: + definition_data = getattr(xblock_class, 'data').default + else: + definition_data = {} + dbmodel = self._create_new_model_data(location.category, location, definition_data, metadata) + xmodule = xblock_class(system, dbmodel) + # force inherited fields w/ defaults to take the defaults so the children can inherit + for attr in INHERITABLE_METADATA: + if hasattr(xmodule, attr): + xmodule._model_data[attr] = getattr(xmodule, attr) + return xmodule - # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so - # if we add one then we need to also add it to the policy information (i.e. metadata) - # we should remove this once we can break this reference from the course to static tabs - if location.category == 'static_tab': - course = self.get_course_for_item(item.location) - existing_tabs = course.tabs or [] - existing_tabs.append({ - 'type': 'static_tab', - 'name': item.display_name, - 'url_slug': item.location.name - }) - course.tabs = existing_tabs - self.update_metadata(course.location, course._model_data._kvs._metadata) - - except pymongo.errors.DuplicateKeyError: - raise DuplicateItemError(location) + def save_xmodule(self, xmodule): + """ + Save the given xmodule (will either create or update based on whether id already exists). + Pulls out the data definition v metadata v children locally but saves it all. + :param xmodule: + """ + # split mongo's persist_dag is more general and useful. + self.collection.save({ + '_id': xmodule.location.dict(), + 'metadata': own_metadata(xmodule), + 'definition': { + 'data': xmodule.xblock_kvs._data, + 'children': xmodule.children if xmodule.has_children else [] + } + }) # recompute (and update) the metadata inheritance tree which is cached - self.refresh_cached_metadata_inheritance_tree(Location(location)) - self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location)) + self.refresh_cached_metadata_inheritance_tree(xmodule.location) + self.fire_updated_modulestore_signal(get_course_id_no_run(xmodule.location), xmodule.location) - return item + def create_and_save_xmodule(self, location, definition_data=None, metadata=None, system=None): + """ + Create the new xmodule and save it. Does not return the new module because if the caller + will insert it as a child, it's inherited metadata will completely change. The difference + between this and just doing create_xmodule and save_xmodule is this ensures static_tabs get + pointed to by the course. + + :param location: a Location--must have a category + :param definition_data: can be empty. The initial definition_data for the kvs + :param metadata: can be empty, the initial metadata for the kvs + :param system: if you already have an xmodule from the course, the xmodule.system value + """ + # differs from split mongo in that I believe most of this logic should be above the persistence + # layer but added it here to enable quick conversion. I'll need to reconcile these. + new_object = self.create_xmodule(location, definition_data, metadata, system) + location = new_object.location + self.save_xmodule(new_object) + + # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so + # if we add one then we need to also add it to the policy information (i.e. metadata) + # we should remove this once we can break this reference from the course to static tabs + # TODO move this special casing to app tier (similar to attaching new element to parent) + if location.category == 'static_tab': + course = self.get_course_for_item(location) + existing_tabs = course.tabs or [] + existing_tabs.append({ + 'type': 'static_tab', + 'name': new_object.display_name, + 'url_slug': new_object.location.name + }) + course.tabs = existing_tabs + self.update_metadata(course.location, course.xblock_kvs._metadata) def fire_updated_modulestore_signal(self, course_id, location): """ @@ -683,7 +744,7 @@ class MongoModuleStore(ModuleStoreBase): if result['n'] == 0: raise ItemNotFoundError(location) - def update_item(self, location, data): + def update_item(self, location, data, allow_not_found=False): """ Set the data in the item specified by the location to data @@ -691,8 +752,11 @@ class MongoModuleStore(ModuleStoreBase): location: Something that can be passed to Location data: A nested dictionary of problem data """ - - self._update_single_item(location, {'definition.data': data}) + try: + self._update_single_item(location, {'definition.data': data}) + except ItemNotFoundError: + if not allow_not_found: + raise def update_children(self, location, children): """ @@ -775,3 +839,24 @@ class MongoModuleStore(ModuleStoreBase): are loaded on demand, rather than up front """ return {} + + def _create_new_model_data(self, category, location, definition_data, metadata): + """ + To instantiate a new xmodule which will be saved latter, set up the dbModel and kvs + """ + kvs = MongoKeyValueStore( + definition_data, + [], + metadata, + location, + category + ) + + class_ = XModuleDescriptor.load_class( + category, + self.default_class + ) + model_data = DbModel(kvs, class_, None, MongoUsage(None, location)) + model_data['category'] = category + model_data['location'] = location + return model_data diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index f34c3a53f9..d289e03739 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -8,11 +8,12 @@ and otherwise returns i4x://org/course/cat/name). from datetime import datetime -from xmodule.modulestore import Location, namedtuple_to_son -from xmodule.modulestore.exceptions import ItemNotFoundError -from xmodule.modulestore.inheritance import own_metadata from xmodule.exceptions import InvalidVersionError -from xmodule.modulestore.mongo.base import MongoModuleStore +from xmodule.modulestore import Location, namedtuple_to_son +from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateItemError +from xmodule.modulestore.inheritance import own_metadata +from xmodule.modulestore.mongo.base import location_to_query, get_course_id_no_run, MongoModuleStore +import pymongo from pytz import UTC DRAFT = 'draft' @@ -92,6 +93,21 @@ class DraftModuleStore(MongoModuleStore): except ItemNotFoundError: return wrap_draft(super(DraftModuleStore, self).get_instance(course_id, location, depth=depth)) + def create_xmodule(self, location, definition_data=None, metadata=None, system=None): + """ + Create the new xmodule but don't save it. Returns the new module with a draft locator + + :param location: a Location--must have a category + :param definition_data: can be empty. The initial definition_data for the kvs + :param metadata: can be empty, the initial metadata for the kvs + :param system: if you already have an xmodule from the course, the xmodule.system value + """ + draft_loc = as_draft(location) + if draft_loc.category in DIRECT_ONLY_CATEGORIES: + raise InvalidVersionError(location) + return super(DraftModuleStore, self).create_xmodule(draft_loc, definition_data, metadata, system) + + def get_items(self, location, course_id=None, depth=0): """ Returns a list of XModuleDescriptor instances for the items @@ -119,14 +135,26 @@ class DraftModuleStore(MongoModuleStore): ] return [wrap_draft(item) for item in draft_items + non_draft_items] - def clone_item(self, source, location): + def convert_to_draft(self, source_location): """ - Clone a new item that is a copy of the item at the location `source` - and writes it to `location` + Create a copy of the source and mark its revision as draft. + + :param source: the location of the source (its revision must be None) """ - if Location(location).category in DIRECT_ONLY_CATEGORIES: - raise InvalidVersionError(location) - return wrap_draft(super(DraftModuleStore, self).clone_item(source, as_draft(location))) + original = self.collection.find_one(location_to_query(source_location)) + draft_location = as_draft(source_location) + if draft_location.category in DIRECT_ONLY_CATEGORIES: + raise InvalidVersionError(source_location) + original['_id'] = draft_location.dict() + try: + self.collection.insert(original) + except pymongo.errors.DuplicateKeyError: + raise DuplicateItemError(original['_id']) + + self.refresh_cached_metadata_inheritance_tree(draft_location) + self.fire_updated_modulestore_signal(get_course_id_no_run(draft_location), draft_location) + + return self._load_items([original])[0] def update_item(self, location, data, allow_not_found=False): """ @@ -140,7 +168,7 @@ class DraftModuleStore(MongoModuleStore): try: draft_item = self.get_item(location) if not getattr(draft_item, 'is_draft', False): - self.clone_item(location, draft_loc) + self.convert_to_draft(location) except ItemNotFoundError, e: if not allow_not_found: raise e @@ -158,7 +186,7 @@ class DraftModuleStore(MongoModuleStore): draft_loc = as_draft(location) draft_item = self.get_item(location) if not getattr(draft_item, 'is_draft', False): - self.clone_item(location, draft_loc) + self.convert_to_draft(location) return super(DraftModuleStore, self).update_children(draft_loc, children) @@ -174,7 +202,7 @@ class DraftModuleStore(MongoModuleStore): draft_item = self.get_item(location) if not getattr(draft_item, 'is_draft', False): - self.clone_item(location, draft_loc) + self.convert_to_draft(location) if 'is_draft' in metadata: del metadata['is_draft'] @@ -218,9 +246,7 @@ class DraftModuleStore(MongoModuleStore): """ Turn the published version into a draft, removing the published version """ - if Location(location).category in DIRECT_ONLY_CATEGORIES: - raise InvalidVersionError(location) - super(DraftModuleStore, self).clone_item(location, as_draft(location)) + self.convert_to_draft(location) super(DraftModuleStore, self).delete_item(location) def _query_children_for_cache_children(self, items): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index c32d0bca4c..564aac141d 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -5,7 +5,6 @@ from django.test import TestCase from django.conf import settings import xmodule.modulestore.django -from xmodule.templates import update_templates from unittest.util import safe_repr @@ -110,22 +109,6 @@ class ModuleStoreTestCase(TestCase): modulestore.collection.remove(query) modulestore.collection.drop() - @staticmethod - def load_templates_if_necessary(): - """ - Load templates into the direct modulestore only if they do not already exist. - We need the templates, because they are copied to create - XModules such as sections and problems. - """ - modulestore = xmodule.modulestore.django.modulestore('direct') - - # Count the number of templates - query = {"_id.course": "templates"} - num_templates = modulestore.collection.find(query).count() - - if num_templates < 1: - update_templates(modulestore) - @classmethod def setUpClass(cls): """ @@ -169,9 +152,6 @@ class ModuleStoreTestCase(TestCase): # Flush anything that is not a template ModuleStoreTestCase.flush_mongo_except_templates() - # Check that we have templates loaded; if not, load them - ModuleStoreTestCase.load_templates_if_necessary() - # Call superclass implementation super(ModuleStoreTestCase, self)._pre_setup() diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 457a88482a..9a0c87ff97 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -1,15 +1,13 @@ -from factory import Factory, lazy_attribute_sequence, lazy_attribute -from uuid import uuid4 import datetime +from factory import Factory, LazyAttributeSequence +from uuid import uuid4 +from pytz import UTC + from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore -from xmodule.modulestore.inheritance import own_metadata -from xmodule.x_module import ModuleSystem -from mitxmako.shortcuts import render_to_string -from xblock.runtime import InvalidScopeError -from pytz import UTC - +from xmodule.course_module import CourseDescriptor +from xblock.core import Scope class XModuleCourseFactory(Factory): """ @@ -21,9 +19,8 @@ class XModuleCourseFactory(Factory): @classmethod def _create(cls, target_class, **kwargs): - template = Location('i4x', 'edx', 'templates', 'course', 'Empty') org = kwargs.pop('org', None) - number = kwargs.pop('number', None) + number = kwargs.pop('number', kwargs.pop('course', None)) display_name = kwargs.pop('display_name', None) location = Location('i4x', org, number, 'course', Location.clean(display_name)) @@ -33,7 +30,7 @@ class XModuleCourseFactory(Factory): store = modulestore() # Write the data to the mongo datastore - new_course = store.clone_item(template, location) + new_course = store.create_xmodule(location) # This metadata code was copied from cms/djangoapps/contentstore/views.py if display_name is not None: @@ -56,13 +53,7 @@ class XModuleCourseFactory(Factory): setattr(new_course, k, v) # Update the data in the mongo datastore - store.update_metadata(new_course.location, own_metadata(new_course)) - store.update_item(new_course.location, new_course._model_data._kvs._data) - - # update_item updates the the course as it exists in the modulestore, but doesn't - # update the instance we are working with, so have to refetch the course after updating it. - new_course = store.get_instance(new_course.id, new_course.location) - + store.save_xmodule(new_course) return new_course @@ -73,7 +64,6 @@ class Course: class CourseFactory(XModuleCourseFactory): FACTORY_FOR = Course - template = 'i4x://edx/templates/course/Empty' org = 'MITx' number = '999' display_name = 'Robot Super Course' @@ -86,18 +76,14 @@ class XModuleItemFactory(Factory): ABSTRACT_FACTORY = True - display_name = None + parent_location = 'i4x://MITx/999/course/Robot_Super_Course' + category = 'problem' + display_name = LazyAttributeSequence(lambda o, n: "{} {}".format(o.category, n)) - @lazy_attribute - def category(attr): - template = Location(attr.template) - return template.category - - @lazy_attribute - def location(attr): - parent = Location(attr.parent_location) - dest_name = attr.display_name.replace(" ", "_") if attr.display_name is not None else uuid4().hex - return parent._replace(category=attr.category, name=dest_name) + @staticmethod + def location(parent, category, display_name): + dest_name = display_name.replace(" ", "_") if display_name is not None else uuid4().hex + return Location(parent).replace(category=category, name=dest_name) @classmethod def _create(cls, target_class, **kwargs): @@ -107,8 +93,7 @@ class XModuleItemFactory(Factory): *parent_location* (required): the location of the parent module (e.g. the parent course or section) - *template* (required): the template to create the item from - (e.g. i4x://templates/section/Empty) + category: the category of the resulting item. *data* (optional): the data for the item (e.g. XML problem definition for a problem item) @@ -121,41 +106,32 @@ class XModuleItemFactory(Factory): """ DETACHED_CATEGORIES = ['about', 'static_tab', 'course_info'] - + # catch any old style users before they get into trouble + assert not 'template' in kwargs parent_location = Location(kwargs.get('parent_location')) - template = Location(kwargs.get('template')) data = kwargs.get('data') + category = kwargs.get('category') display_name = kwargs.get('display_name') metadata = kwargs.get('metadata', {}) + location = kwargs.get('location', XModuleItemFactory.location(parent_location, category, display_name)) + assert location != parent_location store = modulestore('direct') # This code was based off that in cms/djangoapps/contentstore/views.py parent = store.get_item(parent_location) - new_item = store.clone_item(template, kwargs.get('location')) - # replace the display name with an optional parameter passed in from the caller if display_name is not None: - new_item.display_name = display_name + metadata['display_name'] = display_name + # note that location comes from above lazy_attribute + store.create_and_save_xmodule(location, metadata=metadata, definition_data=data) - # Add additional metadata or override current metadata - item_metadata = own_metadata(new_item) - item_metadata.update(metadata) - store.update_metadata(new_item.location.url(), item_metadata) + if location.category not in DETACHED_CATEGORIES: + parent.children.append(location.url()) + store.update_children(parent_location, parent.children) - # replace the data with the optional *data* parameter - if data is not None: - store.update_item(new_item.location, data) - - if new_item.location.category not in DETACHED_CATEGORIES: - store.update_children(parent_location, parent.children + [new_item.location.url()]) - - # update_children updates the the item as it exists in the modulestore, but doesn't - # update the instance we are working with, so have to refetch the item after updating it. - new_item = store.get_item(new_item.location) - - return new_item + return store.get_item(location) class Item: @@ -164,40 +140,4 @@ class Item: class ItemFactory(XModuleItemFactory): FACTORY_FOR = Item - - parent_location = 'i4x://MITx/999/course/Robot_Super_Course' - template = 'i4x://edx/templates/chapter/Empty' - - @lazy_attribute_sequence - def display_name(attr, n): - return "{} {}".format(attr.category.title(), n) - - -def get_test_xmodule_for_descriptor(descriptor): - """ - Attempts to create an xmodule which responds usually correctly from the descriptor. Not guaranteed. - - :param descriptor: - """ - module_sys = ModuleSystem( - ajax_url='', - track_function=None, - get_module=None, - render_template=render_to_string, - replace_urls=None, - xblock_model_data=_test_xblock_model_data_accessor(descriptor) - ) - return descriptor.xmodule(module_sys) - - -def _test_xblock_model_data_accessor(descriptor): - simple_map = {} - for field in descriptor.fields: - try: - simple_map[field.name] = getattr(descriptor, field.name) - except InvalidScopeError: - simple_map[field.name] = field.default - for field in descriptor.module_class.fields: - if field.name not in simple_map: - simple_map[field.name] = field.default - return lambda o: simple_map + category = 'chapter' diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 44e69fb0ed..c149724cc7 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -9,7 +9,6 @@ from xblock.runtime import KeyValueStore, InvalidScopeError from xmodule.modulestore import Location from xmodule.modulestore.mongo import MongoModuleStore, MongoKeyValueStore from xmodule.modulestore.xml_importer import import_from_xml -from xmodule.templates import update_templates from .test_modulestore import check_path_to_location from . import DATA_DIR @@ -51,7 +50,6 @@ class TestMongoModuleStore(object): # Explicitly list the courses to load (don't want the big one) courses = ['toy', 'simple'] import_from_xml(store, DATA_DIR, courses) - update_templates(store) return store @staticmethod @@ -126,7 +124,7 @@ class TestMongoKeyValueStore(object): self.location = Location('i4x://org/course/category/name@version') self.children = ['i4x://org/course/child/a', 'i4x://org/course/child/b'] self.metadata = {'meta': 'meta_val'} - self.kvs = MongoKeyValueStore(self.data, self.children, self.metadata, self.location) + self.kvs = MongoKeyValueStore(self.data, self.children, self.metadata, self.location, 'category') def _check_read(self, key, expected_value): assert_equals(expected_value, self.kvs.get(key)) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 26c8b9bfca..012740ff9a 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -463,7 +463,10 @@ class XMLModuleStore(ModuleStoreBase): # tabs are referenced in policy.json through a 'slug' which is just the filename without the .html suffix slug = os.path.splitext(os.path.basename(filepath))[0] loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, category, slug) - module = HtmlDescriptor(system, {'data': html, 'location': loc}) + module = HtmlDescriptor( + system, + {'data': html, 'location': loc, 'category': category} + ) # VS[compat]: # Hack because we need to pull in the 'display_name' for static tabs (because we need to edit them) # from the course policy 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..60eb9dc749 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 @@ -810,7 +810,6 @@ class CombinedOpenEndedV1Descriptor(): filename_extension = "xml" has_score = True - template_dir_name = "combinedopenended" def __init__(self, system): self.system = system diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index 0f0851fbf7..8d8a85f788 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -730,7 +730,6 @@ class OpenEndedDescriptor(): filename_extension = "xml" has_score = True - template_dir_name = "openended" def __init__(self, system): self.system = system diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py index a5498289e2..1262e1f68f 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py @@ -287,7 +287,6 @@ class SelfAssessmentDescriptor(): filename_extension = "xml" has_score = True - template_dir_name = "selfassessment" def __init__(self, system): self.system = system diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index c88a2e1b38..03003ed1e5 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -613,7 +613,6 @@ class PeerGradingDescriptor(PeerGradingFields, RawDescriptor): has_score = True always_recalculate_grades = True - template_dir_name = "peer_grading" #Specify whether or not to pass in open ended interface needs_open_ended_interface = True diff --git a/common/lib/xmodule/xmodule/poll_module.py b/common/lib/xmodule/xmodule/poll_module.py index ca12f239ab..8e7407752a 100644 --- a/common/lib/xmodule/xmodule/poll_module.py +++ b/common/lib/xmodule/xmodule/poll_module.py @@ -140,7 +140,6 @@ class PollDescriptor(PollFields, MakoModuleDescriptor, XmlDescriptor): _child_tag_name = 'answer' module_class = PollModule - template_dir_name = 'poll' @classmethod def definition_from_xml(cls, xml_object, system): diff --git a/common/lib/xmodule/xmodule/templates.py b/common/lib/xmodule/xmodule/templates.py index 6479b3df24..8e350bb618 100644 --- a/common/lib/xmodule/xmodule/templates.py +++ b/common/lib/xmodule/xmodule/templates.py @@ -1,34 +1,18 @@ """ -This module handles loading xmodule templates from disk into the modulestore. -These templates are used by the CMS to provide baseline content that -can be cloned when adding new modules to a course. +This module handles loading xmodule templates +These templates are used by the CMS to provide content that overrides xmodule defaults for +samples. -`Template`s are defined in x_module. They contain 3 attributes: - metadata: A dictionary with the template metadata. This should contain - any values for fields - * with scope Scope.settings - * that have values different than the field defaults - * and that are to be editable in Studio - data: A JSON value that defines the template content. This should be a dictionary - containing values for fields - * with scope Scope.content - * that have values different than the field defaults - * and that are to be editable in Studio - or, if the module uses a single Scope.content String field named `data`, this - should be a string containing the contents of that field - children: A list of Location urls that define the template children - -Templates are defined on XModuleDescriptor types, in the template attribute. +``Template``s are defined in x_module. They contain 2 attributes: + :metadata: A dictionary with the template metadata + :data: A JSON value that defines the template content """ - +# should this move to cms since it's really only for module crud? import logging -from fs.memoryfs import MemoryFS from collections import defaultdict from .x_module import XModuleDescriptor -from .mako_module import MakoDescriptorSystem -from .modulestore import Location log = logging.getLogger(__name__) @@ -37,73 +21,9 @@ def all_templates(): """ Returns all templates for enabled modules, grouped by descriptor type """ - + # TODO use memcache to memoize w/ expiration templates = defaultdict(list) for category, descriptor in XModuleDescriptor.load_classes(): templates[category] = descriptor.templates() return templates - - -class TemplateTestSystem(MakoDescriptorSystem): - """ - This system exists to help verify that XModuleDescriptors can be instantiated - from their defined templates before we load the templates into the modulestore. - """ - def __init__(self): - super(TemplateTestSystem, self).__init__( - lambda *a, **k: None, - MemoryFS(), - lambda msg: None, - render_template=lambda *a, **k: None, - ) - - -def update_templates(modulestore): - """ - Updates the set of templates in the modulestore with all templates currently - available from the installed plugins - """ - - # cdodge: build up a list of all existing templates. This will be used to determine which - # templates have been removed from disk - and thus we need to remove from the DB - templates_to_delete = modulestore.get_items(['i4x', 'edx', 'templates', None, None, None]) - - for category, templates in all_templates().items(): - for template in templates: - if 'display_name' not in template.metadata: - log.warning('No display_name specified in template {0}, skipping'.format(template)) - continue - - template_location = Location('i4x', 'edx', 'templates', category, Location.clean_for_url_name(template.metadata['display_name'])) - - try: - json_data = { - 'definition': { - 'data': template.data, - 'children': template.children - }, - 'metadata': template.metadata - } - json_data['location'] = template_location.dict() - - XModuleDescriptor.load_from_json(json_data, TemplateTestSystem()) - except: - log.warning('Unable to instantiate {cat} from template {template}, skipping'.format( - cat=category, - template=template - ), exc_info=True) - continue - - modulestore.update_item(template_location, template.data) - modulestore.update_children(template_location, template.children) - modulestore.update_metadata(template_location, template.metadata) - - # remove template from list of templates to delete - templates_to_delete = [t for t in templates_to_delete if t.location != template_location] - - # now remove all templates which appear to have removed from disk - if len(templates_to_delete) > 0: - logging.debug('deleting dangling templates = {0}'.format(templates_to_delete)) - for template in templates_to_delete: - modulestore.delete_item(template.location) diff --git a/common/lib/xmodule/xmodule/templates/about/empty.yaml b/common/lib/xmodule/xmodule/templates/about/empty.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/about/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/annotatable/default.yaml b/common/lib/xmodule/xmodule/templates/annotatable/default.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/annotatable/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml b/common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/course/empty.yaml b/common/lib/xmodule/xmodule/templates/course/empty.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/course/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml b/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/default/empty.yaml b/common/lib/xmodule/xmodule/templates/default/empty.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/default/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/discussion/default.yaml b/common/lib/xmodule/xmodule/templates/discussion/default.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/discussion/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/html/announcement.yaml b/common/lib/xmodule/xmodule/templates/html/announcement.yaml index 30a8ccb41e..c0ecc61524 100644 --- a/common/lib/xmodule/xmodule/templates/html/announcement.yaml +++ b/common/lib/xmodule/xmodule/templates/html/announcement.yaml @@ -21,4 +21,3 @@ data: | -children: [] diff --git a/common/lib/xmodule/xmodule/templates/html/empty.yaml b/common/lib/xmodule/xmodule/templates/html/empty.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/html/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/html/everything.yaml b/common/lib/xmodule/xmodule/templates/html/everything.yaml deleted file mode 100644 index 348ce64fa1..0000000000 --- a/common/lib/xmodule/xmodule/templates/html/everything.yaml +++ /dev/null @@ -1,33 +0,0 @@ ---- -metadata: - display_name: Announcement - -data: | -

      Heading of document

      -

      First subheading

      -

      This is a paragraph. It will take care of line breaks for you.

      HTML only parses the location - - of tags for inserting line breaks into your doc, not - line - breaks - you - add - yourself. -

      -

      Links

      -

      You can refer to other parts of the internet with a link, to other parts of your course by prepending your link with /course/

      -

      Now a list:

      -
        -
      • An item
        • -
      • Another item
        • -
      • And yet another
        • -
      -

      This list has an ordering

      -
        -
      1. An item
        2. -
      2. Another item
        3. -
      3. Yet another item
        4. -
      -

      Note, we have a lot of standard edX styles, so please try to avoid any custom styling, and make sure that you make a note of any custom styling that you do yourself so that we can incorporate it into - tools that other people can use.

      -children: [] diff --git a/common/lib/xmodule/xmodule/templates/html/latex_html.yaml b/common/lib/xmodule/xmodule/templates/html/latex_html.yaml index ba5c4b5c06..2db7e98c65 100644 --- a/common/lib/xmodule/xmodule/templates/html/latex_html.yaml +++ b/common/lib/xmodule/xmodule/templates/html/latex_html.yaml @@ -19,4 +19,3 @@ data: | It is very convenient to write complex equations in LaTeX.

      -children: [] diff --git a/common/lib/xmodule/xmodule/templates/peer_grading/default.yaml b/common/lib/xmodule/xmodule/templates/peer_grading/default.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/peer_grading/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml b/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml index 3b051f2ba8..1717bb91ad 100644 --- a/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml @@ -1,9 +1,9 @@ - --- metadata: display_name: Circuit Schematic Builder rerandomize: never showanswer: finished + markdown: !!null data: | Please make a voltage divider that splits the provided voltage evenly. diff --git a/common/lib/xmodule/xmodule/templates/problem/customgrader.yaml b/common/lib/xmodule/xmodule/templates/problem/customgrader.yaml index 48feef481b..05de74f28c 100644 --- a/common/lib/xmodule/xmodule/templates/problem/customgrader.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/customgrader.yaml @@ -1,50 +1,47 @@ --- metadata: display_name: Custom Python-Evaluated Input - rerandomize: never - showanswer: finished + markdown: !!null data: | - -

      - A custom python-evaluated input problem accepts one or more lines of text input from the - student, and evaluates the inputs for correctness based on evaluation using a - python script embedded within the problem. -

      + +

      + A custom python-evaluated input problem accepts one or more lines of text input from the + student, and evaluates the inputs for correctness based on evaluation using a + python script embedded within the problem. +

      - + -

      Enter two integers which sum to 10:

      - -
      - -       +

      Enter two integers which sum to 10:

      + +
      + +       -

      Enter two integers which sum to 20:

      - -
      - -       - - -
      -

      Explanation

      -

      Any set of integers on the line \(y = 10 - x\) and \(y = 20 - x\) satisfy these constraints.

      - -
      -       -       - -children: [] +

      Enter two integers which sum to 20:

      + +
      + +       + + +
      +

      Explanation

      +

      Any set of integers on the line \(y = 10 - x\) and \(y = 20 - x\) satisfy these constraints.

      + +
      +       +       diff --git a/common/lib/xmodule/xmodule/templates/problem/empty.yaml b/common/lib/xmodule/xmodule/templates/problem/empty.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/problem/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/problem/emptyadvanced.yaml b/common/lib/xmodule/xmodule/templates/problem/emptyadvanced.yaml deleted file mode 100644 index 3d696ec2fd..0000000000 --- a/common/lib/xmodule/xmodule/templates/problem/emptyadvanced.yaml +++ /dev/null @@ -1,10 +0,0 @@ ---- -metadata: - display_name: Blank Advanced Problem - rerandomize: never - showanswer: finished -data: | - - - -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/forumularesponse.yaml b/common/lib/xmodule/xmodule/templates/problem/forumularesponse.yaml index 0401a01c31..4cf877bd1f 100644 --- a/common/lib/xmodule/xmodule/templates/problem/forumularesponse.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/forumularesponse.yaml @@ -3,6 +3,7 @@ metadata: display_name: Math Expression Input rerandomize: never showanswer: finished + markdown: !!null data: |

      @@ -43,5 +44,3 @@ data: | - -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml b/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml index ab1f22e3b2..566997671d 100644 --- a/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml @@ -3,6 +3,7 @@ metadata: display_name: Image Mapped Input rerandomize: never showanswer: finished + markdown: !!null data: |

      @@ -21,6 +22,3 @@ data: | - - -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/latex_problem.yaml b/common/lib/xmodule/xmodule/templates/problem/latex_problem.yaml index 82d7e8c1ae..097055cfe3 100644 --- a/common/lib/xmodule/xmodule/templates/problem/latex_problem.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/latex_problem.yaml @@ -85,6 +85,7 @@ metadata: can contain equations: $\alpha = \frac{2}{\sqrt{1+\gamma}}$ } This is some text after the showhide example. + markdown: !!null data: | @@ -214,4 +215,3 @@ data: |

      -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/multiplechoice.yaml b/common/lib/xmodule/xmodule/templates/problem/multiplechoice.yaml index 10d51de280..202fc03b44 100644 --- a/common/lib/xmodule/xmodule/templates/problem/multiplechoice.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/multiplechoice.yaml @@ -3,33 +3,25 @@ metadata: display_name: Multiple Choice rerandomize: never showanswer: finished - markdown: - "A multiple choice problem presents radio buttons for student input. Students can only select a single + markdown: | + A multiple choice problem presents radio buttons for student input. Students can only select a single option presented. Multiple Choice questions have been the subject of many areas of research due to the early invention and adoption of bubble sheets. - One of the main elements that goes into a good multiple choice question is the existence of good distractors. That is, each of the alternate responses presented to the student should be the result of a plausible mistake that a student might make. - What Apple device competed with the portable CD player? - ( ) The iPad - ( ) Napster - (x) The iPod - ( ) The vegetable peeler - [explanation] The release of the iPod allowed consumers to carry their entire music library with them in a format that did not rely on fragile and energy-intensive spinning disks. [explanation] - " data: |

      @@ -54,4 +46,3 @@ data: | -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/numericalresponse.yaml b/common/lib/xmodule/xmodule/templates/problem/numericalresponse.yaml index 548fd94fab..9b2ddec2a7 100644 --- a/common/lib/xmodule/xmodule/templates/problem/numericalresponse.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/numericalresponse.yaml @@ -3,43 +3,33 @@ metadata: display_name: Numerical Input rerandomize: never showanswer: finished - markdown: - "A numerical input problem accepts a line of text input from the + markdown: | + A numerical input problem accepts a line of text input from the student, and evaluates the input for correctness based on its numerical value. - The answer is correct if it is within a specified numerical tolerance of the expected answer. - Enter the numerical value of Pi: - = 3.14159 +- .02 - Enter the approximate value of 502*9: - = 4518 +- 15% - - + Enter the number of fingers on a human hand: - = 5 - [explanation] Pi, or the the ratio between a circle's circumference to its diameter, is an irrational number known to extreme precision. It is value is approximately equal to 3.14. - + Although you can get an exact value by typing 502*9 into a calculator, the result will be close to 500*10, or 5,000. The grader accepts any response within 15% of the true value, 4518, so that you can use any estimation technique that you like. - + If you look at your hand, you can count that you have five fingers. [explanation] - " - data: |

      @@ -83,5 +73,3 @@ data: | - -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/optionresponse.yaml b/common/lib/xmodule/xmodule/templates/problem/optionresponse.yaml index c2edfb1cbc..8e59f8ae4d 100644 --- a/common/lib/xmodule/xmodule/templates/problem/optionresponse.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/optionresponse.yaml @@ -3,19 +3,16 @@ metadata: display_name: Dropdown rerandomize: never showanswer: finished - markdown: - "Dropdown problems give a limited set of options for students to respond with, and present those options + markdown: | + Dropdown problems give a limited set of options for students to respond with, and present those options in a format that encourages them to search for a specific answer rather than being immediately presented with options from which to recognize the correct answer. - The answer options and the identification of the correct answer is defined in the optioninput tag. - Translation between Dropdown and __________ is extremely straightforward: - - [[(Multiple Choice), Text Input, Numerical Input, External Response, Image Response]] + [[(Multiple Choice), Text Input, Numerical Input, External Response, Image Response]] [explanation] Multiple Choice also allows students to select from a variety of pre-written responses, although the @@ -23,7 +20,6 @@ metadata: slightly because students are more likely to think of an answer and then search for it rather than relying purely on recognition to answer the question. [explanation] - " data: |

      Dropdown problems give a limited set of options for students to respond with, and present those options @@ -45,4 +41,3 @@ data: | -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/problem_with_hint.yaml b/common/lib/xmodule/xmodule/templates/problem/problem_with_hint.yaml index 73a94ed941..0d93cd3c5e 100644 --- a/common/lib/xmodule/xmodule/templates/problem/problem_with_hint.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/problem_with_hint.yaml @@ -46,7 +46,7 @@ metadata: enter your answer in upper or lower case, with or without quotes. \edXabox{type="custom" cfn='test_str' expect='python' hintfn='hint_fn'} - + markdown: !!null data: | @@ -92,4 +92,3 @@ data: |

      -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/string_response.yaml b/common/lib/xmodule/xmodule/templates/problem/string_response.yaml index 64e3dc062f..9c59ae3bc2 100644 --- a/common/lib/xmodule/xmodule/templates/problem/string_response.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/string_response.yaml @@ -3,16 +3,13 @@ metadata: display_name: Text Input rerandomize: never showanswer: finished - # Note, the extra newlines are needed to make the yaml parser add blank lines instead of folding - markdown: - "A text input problem accepts a line of text from the + markdown: | + A text input problem accepts a line of text from the student, and evaluates the input for correctness based on an expected answer. - The answer is correct if it matches every character of the expected answer. This can be a problem with international spelling, dates, or anything where the format of the answer is not clear. - Which US state has Lansing as its capital? @@ -23,9 +20,8 @@ metadata: Lansing is the capital of Michigan, although it is not Michgan's largest city, or even the seat of the county in which it resides. [explanation] - " data: | - +

      A text input problem accepts a line of text from the @@ -46,4 +42,3 @@ data: | -children: [] diff --git a/common/lib/xmodule/xmodule/templates/sequence/with_video.yaml b/common/lib/xmodule/xmodule/templates/sequence/with_video.yaml deleted file mode 100644 index a56d44ebff..0000000000 --- a/common/lib/xmodule/xmodule/templates/sequence/with_video.yaml +++ /dev/null @@ -1,7 +0,0 @@ ---- -metadata: - display_name: Sequence with Video - data_dir: a_made_up_name -data: '' -children: - - 'i4x://edx/templates/video/default' diff --git a/common/lib/xmodule/xmodule/templates/statictab/empty.yaml b/common/lib/xmodule/xmodule/templates/statictab/empty.yaml deleted file mode 100644 index 9e26dfeeb6..0000000000 --- a/common/lib/xmodule/xmodule/templates/statictab/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/templates/video/default.yaml b/common/lib/xmodule/xmodule/templates/video/default.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/video/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/videoalpha/default.yaml b/common/lib/xmodule/xmodule/templates/videoalpha/default.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/videoalpha/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/word_cloud/default.yaml b/common/lib/xmodule/xmodule/templates/word_cloud/default.yaml deleted file mode 100644 index 0967ef424b..0000000000 --- a/common/lib/xmodule/xmodule/templates/word_cloud/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/tests/test_logic.py b/common/lib/xmodule/xmodule/tests/test_logic.py index 9be533885c..5fe7aa2832 100644 --- a/common/lib/xmodule/xmodule/tests/test_logic.py +++ b/common/lib/xmodule/xmodule/tests/test_logic.py @@ -28,7 +28,8 @@ class LogicTest(unittest.TestCase): def setUp(self): class EmptyClass: """Empty object.""" - pass + url_name = '' + category = 'test' self.system = get_test_system() self.descriptor = EmptyClass() diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index 7ccc71dd96..a277ff2900 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -141,6 +141,7 @@ class EditableMetadataFieldsTest(unittest.TestCase): def get_xml_editable_fields(self, model_data): system = get_test_system() system.render_template = Mock(return_value="

      Test Template HTML
      ") + model_data['category'] = 'test' return XmlDescriptor(runtime=system, model_data=model_data).editable_metadata_fields def get_descriptor(self, model_data): diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index ebff888f34..381cc9a622 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -97,7 +97,6 @@ class VideoDescriptor(VideoFields, MetadataOnlyEditingDescriptor, RawDescriptor): module_class = VideoModule - template_dir_name = "video" def __init__(self, *args, **kwargs): super(VideoDescriptor, self).__init__(*args, **kwargs) diff --git a/common/lib/xmodule/xmodule/videoalpha_module.py b/common/lib/xmodule/xmodule/videoalpha_module.py index 33945c33fc..d8ed8949f1 100644 --- a/common/lib/xmodule/xmodule/videoalpha_module.py +++ b/common/lib/xmodule/xmodule/videoalpha_module.py @@ -179,4 +179,3 @@ class VideoAlphaModule(VideoAlphaFields, XModule): class VideoAlphaDescriptor(VideoAlphaFields, RawDescriptor): """Descriptor for `VideoAlphaModule`.""" module_class = VideoAlphaModule - template_dir_name = "videoalpha" diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 0528bbfb6c..882e308c77 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -356,6 +356,7 @@ class XmlDescriptor(XModuleDescriptor): if key not in set(f.name for f in cls.fields + cls.lms.fields): model_data['xml_attributes'][key] = value model_data['location'] = location + model_data['category'] = xml_object.tag return cls( system, diff --git a/lms/djangoapps/courseware/features/common.py b/lms/djangoapps/courseware/features/common.py index 0aa079ebac..55e82e0e90 100644 --- a/lms/djangoapps/courseware/features/common.py +++ b/lms/djangoapps/courseware/features/common.py @@ -10,7 +10,6 @@ from django.contrib.auth.models import User from student.models import CourseEnrollment from xmodule.modulestore import Location from xmodule.modulestore.django import _MODULESTORES, modulestore -from xmodule.templates import update_templates from xmodule.course_module import CourseDescriptor from courseware.courses import get_course_by_id from xmodule import seq_module, vertical_module @@ -39,7 +38,7 @@ def create_course(step, course): display_name='Test Section') problem_section = world.ItemFactory.create(parent_location=world.scenario_dict['SECTION'].location, - template='i4x://edx/templates/sequential/Empty', + category='sequential' display_name='Test Section') @@ -62,7 +61,7 @@ def i_am_registered_for_the_course(step, course): @step(u'The course "([^"]*)" has extra tab "([^"]*)"$') def add_tab_to_course(step, course, extra_tab_name): section_item = world.ItemFactory.create(parent_location=course_location(course), - template="i4x://edx/templates/static_tab/Empty", + category="static_tab", display_name=str(extra_tab_name)) diff --git a/lms/djangoapps/courseware/features/navigation.py b/lms/djangoapps/courseware/features/navigation.py index 7c2474ae1a..c87e6122a4 100644 --- a/lms/djangoapps/courseware/features/navigation.py +++ b/lms/djangoapps/courseware/features/navigation.py @@ -24,11 +24,11 @@ def view_course_multiple_sections(step): display_name=section_name(2)) place1 = world.ItemFactory.create(parent_location=section1.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name=subsection_name(1)) place2 = world.ItemFactory.create(parent_location=section2.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name=subsection_name(2)) add_problem_to_course_section('model_course', 'multiple choice', place1.location) @@ -46,7 +46,7 @@ def view_course_multiple_subsections(step): display_name=section_name(1)) place1 = world.ItemFactory.create(parent_location=section1.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name=subsection_name(1)) place2 = world.ItemFactory.create(parent_location=section1.location, @@ -66,7 +66,7 @@ def view_course_multiple_sequences(step): display_name=section_name(1)) place1 = world.ItemFactory.create(parent_location=section1.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name=subsection_name(1)) add_problem_to_course_section('model_course', 'multiple choice', place1.location) @@ -177,9 +177,8 @@ def add_problem_to_course_section(course, problem_type, parent_location, extraMe # Create a problem item using our generated XML # We set rerandomize=always in the metadata so that the "Reset" button # will appear. - template_name = "i4x://edx/templates/problem/Blank_Common_Problem" world.ItemFactory.create(parent_location=parent_location, - template=template_name, + category='problem', display_name=str(problem_type), data=problem_xml, metadata=metadata) diff --git a/lms/djangoapps/courseware/features/problems.py b/lms/djangoapps/courseware/features/problems.py index 82bb4959a8..e0c3c004da 100644 --- a/lms/djangoapps/courseware/features/problems.py +++ b/lms/djangoapps/courseware/features/problems.py @@ -17,7 +17,7 @@ def view_problem_with_attempts(step, problem_type, attempts): i_am_registered_for_the_course(step, 'model_course') # Ensure that the course has this problem type - add_problem_to_course(world.scenario_dict['COURSE'].number, problem_type, {'attempts': attempts}) + add_problem_to_course(world.scenario_dict['COURSE'].number, problem_type, {'max_attempts': attempts}) # Go to the one section in the factory-created course # which should be loaded with the correct problem diff --git a/lms/djangoapps/courseware/features/problems_setup.py b/lms/djangoapps/courseware/features/problems_setup.py index 1805da55d0..6086d7fa5e 100644 --- a/lms/djangoapps/courseware/features/problems_setup.py +++ b/lms/djangoapps/courseware/features/problems_setup.py @@ -273,9 +273,9 @@ def add_problem_to_course(course, problem_type, extraMeta=None): # Create a problem item using our generated XML # We set rerandomize=always in the metadata so that the "Reset" button # will appear. - template_name = "i4x://edx/templates/problem/Blank_Common_Problem" - world.ItemFactory.create(parent_location=section_location(course), - template=template_name, + category_name = "problem" + return world.ItemFactory.create(parent_location=section_location(course), + category=category_name, display_name=str(problem_type), data=problem_xml, metadata=metadata) diff --git a/lms/djangoapps/courseware/features/video.py b/lms/djangoapps/courseware/features/video.py index 6b05af51b5..f95ffd9917 100644 --- a/lms/djangoapps/courseware/features/video.py +++ b/lms/djangoapps/courseware/features/video.py @@ -43,14 +43,13 @@ def view_videoalpha(step): def add_video_to_course(course): - template_name = 'i4x://edx/templates/video/default' world.ItemFactory.create(parent_location=section_location(course), - template=template_name, + category='video', display_name='Video') def add_videoalpha_to_course(course): - template_name = 'i4x://edx/templates/videoalpha/Video_Alpha' + category = 'videoalpha' world.ItemFactory.create(parent_location=section_location(course), - template=template_name, + category=category, display_name='Video Alpha') diff --git a/lms/djangoapps/courseware/tests/__init__.py b/lms/djangoapps/courseware/tests/__init__.py index 0abbaa02cf..31fe376d69 100644 --- a/lms/djangoapps/courseware/tests/__init__.py +++ b/lms/djangoapps/courseware/tests/__init__.py @@ -29,17 +29,17 @@ class BaseTestXmodule(ModuleStoreTestCase): 2. create, enrol and login users for this course; Any xmodule should overwrite only next parameters for test: - 1. TEMPLATE_NAME + 1. CATEGORY 2. DATA 3. MODEL_DATA - This class should not contain any tests, because TEMPLATE_NAME + This class should not contain any tests, because CATEGORY should be defined in child class. """ USER_COUNT = 2 # Data from YAML common/lib/xmodule/xmodule/templates/NAME/default.yaml - TEMPLATE_NAME = "" + CATEGORY = "" DATA = '' MODEL_DATA = {'data': ''} @@ -53,11 +53,11 @@ class BaseTestXmodule(ModuleStoreTestCase): chapter = ItemFactory.create( parent_location=self.course.location, - template="i4x://edx/templates/sequential/Empty", + category="sequential", ) section = ItemFactory.create( parent_location=chapter.location, - template="i4x://edx/templates/sequential/Empty" + category="sequential" ) # username = robot{0}, password = 'test' @@ -71,7 +71,7 @@ class BaseTestXmodule(ModuleStoreTestCase): self.item_descriptor = ItemFactory.create( parent_location=section.location, - template=self.TEMPLATE_NAME, + category=self.CATEGORY, data=self.DATA ) diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 83ae7dc73e..7e9b55a4fb 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -130,7 +130,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase): problem = ItemFactory.create( parent_location=section_location, - template=problem_template, + category='problem', data=prob_xml, metadata={'randomize': 'always'}, display_name=name @@ -149,13 +149,13 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase): if not(hasattr(self, 'chapter')): self.chapter = ItemFactory.create( parent_location=self.course.location, - template="i4x://edx/templates/chapter/Empty", + category='chapter' ) section = ItemFactory.create( parent_location=self.chapter.location, display_name=name, - template="i4x://edx/templates/sequential/Empty", + category='sequential', metadata={'graded': True, 'format': section_format} ) diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index a0fdecc77a..829308423c 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -7,7 +7,7 @@ from . import BaseTestXmodule class TestVideo(BaseTestXmodule): """Integration tests: web client + mongo.""" - TEMPLATE_NAME = "i4x://edx/templates/video/default" + TEMPLATE_NAME = "video" DATA = '