diff --git a/cms/djangoapps/contentstore/tests/test_checklists.py b/cms/djangoapps/contentstore/tests/test_checklists.py index 5a99c37fbb..3b26f6d7d9 100644 --- a/cms/djangoapps/contentstore/tests/test_checklists.py +++ b/cms/djangoapps/contentstore/tests/test_checklists.py @@ -1,5 +1,6 @@ """ Unit tests for checklist methods in views.py. """ from contentstore.utils import get_modulestore +from contentstore.views.checklist import expand_checklist_action_url from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.tests.factories import CourseFactory from django.core.urlresolvers import reverse @@ -22,20 +23,16 @@ class ChecklistTestCase(CourseTestCase): def compare_checklists(self, persisted, request): """ Handles url expansion as possible difference and descends into guts - :param persisted: - :param request: """ self.assertEqual(persisted['short_description'], request['short_description']) - compare_urls = (persisted.get('action_urls_expanded') == request.get('action_urls_expanded')) - pers, req = None, None - for pers, req in zip(persisted['items'], request['items']): + expanded_checklist = expand_checklist_action_url(self.course, persisted) + for pers, req in zip(expanded_checklist['items'], request['items']): self.assertEqual(pers['short_description'], req['short_description']) - self.assertEqual(pers['long_description'], req['long_description']) - self.assertEqual(pers['is_checked'], req['is_checked']) - if compare_urls: + self.assertEqual(pers['long_description'], req['long_description']) + self.assertEqual(pers['is_checked'], req['is_checked']) self.assertEqual(pers['action_url'], req['action_url']) - self.assertEqual(pers['action_text'], req['action_text']) - self.assertEqual(pers['action_external'], req['action_external']) + self.assertEqual(pers['action_text'], req['action_text']) + self.assertEqual(pers['action_external'], req['action_external']) def test_get_checklists(self): """ Tests the get checklists method. """ @@ -46,6 +43,11 @@ class ChecklistTestCase(CourseTestCase): }) response = self.client.get(checklists_url) self.assertContains(response, "Getting Started With Studio") + # Verify expansion of action URL happened. + self.assertContains(response, '/mitX/333/team/Checklists_Course') + # Verify persisted checklist does NOT have expanded URL. + checklist_0 = self.get_persisted_checklists()[0] + self.assertEqual('ManageUsers', get_action_url(checklist_0, 0)) payload = response.content # Now delete the checklists from the course and verify they get repopulated (for courses @@ -67,7 +69,11 @@ class ChecklistTestCase(CourseTestCase): 'name': self.course.location.name}) returned_checklists = json.loads(self.client.get(update_url).content) - for pay, resp in zip(self.get_persisted_checklists(), returned_checklists): + # Verify that persisted checklists do not have expanded action URLs. + # compare_checklists will verify that returned_checklists DO have expanded action URLs. + pers = self.get_persisted_checklists() + self.assertEqual('CourseOutline', get_first_item(pers[1]).get('action_url')) + for pay, resp in zip(pers, returned_checklists): self.compare_checklists(pay, resp) def test_update_checklists_index_ignored_on_get(self): @@ -103,19 +109,21 @@ class ChecklistTestCase(CourseTestCase): update_url = reverse('checklists_updates', kwargs={'org': self.course.location.org, 'course': self.course.location.course, 'name': self.course.location.name, - 'checklist_index': 2}) + 'checklist_index': 1}) - def get_first_item(checklist): - return checklist['items'][0] - - payload = self.course.checklists[2] + payload = self.course.checklists[1] self.assertFalse(get_first_item(payload).get('is_checked')) + self.assertEqual('CourseOutline', get_first_item(payload).get('action_url')) get_first_item(payload)['is_checked'] = True returned_checklist = json.loads(self.client.post(update_url, json.dumps(payload), "application/json").content) self.assertTrue(get_first_item(returned_checklist).get('is_checked')) - pers = self.get_persisted_checklists() - self.compare_checklists(pers[2], returned_checklist) + persisted_checklist = self.get_persisted_checklists()[1] + # Verify that persisted checklist does not have expanded action URLs. + # compare_checklists will verify that returned_checklist DOES have expanded action URLs. + self.assertEqual('CourseOutline', get_first_item(persisted_checklist).get('action_url')) + self.compare_checklists(persisted_checklist, returned_checklist) + def test_update_checklists_delete_unsupported(self): """ Delete operation is not supported. """ @@ -125,3 +133,36 @@ class ChecklistTestCase(CourseTestCase): 'checklist_index': 100}) response = self.client.delete(update_url) self.assertEqual(response.status_code, 405) + + def test_expand_checklist_action_url(self): + """ + Tests the method to expand checklist action url. + """ + + def test_expansion(checklist, index, stored, expanded): + """ + Tests that the expected expanded value is returned for the item at the given index. + + Also verifies that the original checklist is not modified. + """ + self.assertEqual(get_action_url(checklist, index), stored) + expanded_checklist = expand_checklist_action_url(self.course, checklist) + self.assertEqual(get_action_url(expanded_checklist, index), expanded) + # Verify no side effect in the original list. + self.assertEqual(get_action_url(checklist, index), stored) + + test_expansion(self.course.checklists[0], 0, 'ManageUsers', '/mitX/333/team/Checklists_Course') + test_expansion(self.course.checklists[1], 1, 'CourseOutline', '/mitX/333/course/Checklists_Course') + test_expansion(self.course.checklists[2], 0, 'http://help.edge.edx.org/', 'http://help.edge.edx.org/') + + +def get_first_item(checklist): + """ Returns the first item from the checklist. """ + return checklist['items'][0] + + +def get_action_url(checklist, index): + """ + Returns the action_url for the item at the specified index in the given checklist. + """ + return checklist['items'][index]['action_url'] diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py index 030aa70693..773ec087eb 100644 --- a/cms/djangoapps/contentstore/views/checklist.py +++ b/cms/djangoapps/contentstore/views/checklist.py @@ -1,4 +1,5 @@ import json +import copy from util.json_request import JsonResponse from django.http import HttpResponseBadRequest @@ -32,19 +33,16 @@ def get_checklists(request, org, course, name): # If course was created before checklists were introduced, copy them over # from the template. - copied = False if not course_module.checklists: course_module.checklists = CourseDescriptor.checklists.default - copied = True - - checklists, modified = expand_checklist_action_urls(course_module) - if copied or modified: course_module.save() modulestore.update_metadata(location, own_metadata(course_module)) + + expanded_checklists = expand_all_action_urls(course_module) return render_to_response('checklists.html', { 'context_course': course_module, - 'checklists': checklists + 'checklists': expanded_checklists }) @@ -68,14 +66,20 @@ def update_checklist(request, org, course, name, checklist_index=None): if request.method in ("POST", "PUT"): if checklist_index is not None and 0 <= int(checklist_index) < len(course_module.checklists): index = int(checklist_index) - course_module.checklists[index] = json.loads(request.body) + persisted_checklist = course_module.checklists[index] + modified_checklist = json.loads(request.body) + # Only thing the user can modify is the "checked" state. + # We don't want to persist what comes back from the client because it will + # include the expanded action URLs (which are non-portable). + for item_index, item in enumerate(modified_checklist.get('items')): + persisted_checklist['items'][item_index]['is_checked'] = item['is_checked'] # seeming noop which triggers kvs to record that the metadata is # not default course_module.checklists = course_module.checklists - checklists, _ = expand_checklist_action_urls(course_module) course_module.save() modulestore.update_metadata(location, own_metadata(course_module)) - return JsonResponse(checklists[index]) + expanded_checklist = expand_checklist_action_url(course_module, persisted_checklist) + return JsonResponse(expanded_checklist) else: return HttpResponseBadRequest( ( "Could not save checklist state because the checklist index " @@ -85,23 +89,30 @@ def update_checklist(request, org, course, name, checklist_index=None): elif request.method == 'GET': # In the JavaScript view initialize method, we do a fetch to get all # the checklists. - checklists, modified = expand_checklist_action_urls(course_module) - if modified: - course_module.save() - modulestore.update_metadata(location, own_metadata(course_module)) - return JsonResponse(checklists) + expanded_checklists = expand_all_action_urls(course_module) + return JsonResponse(expanded_checklists) -def expand_checklist_action_urls(course_module): +def expand_all_action_urls(course_module): """ - Gets the checklists out of the course module and expands their action urls - if they have not yet been expanded. + Gets the checklists out of the course module and expands their action urls. - Returns the checklists with modified urls, as well as a boolean - indicating whether or not the checklists were modified. + Returns a copy of the checklists with modified urls, without modifying the persisted version + of the checklists. """ - checklists = course_module.checklists - modified = False + expanded_checklists = [] + for checklist in course_module.checklists: + expanded_checklists.append(expand_checklist_action_url(course_module, checklist)) + return expanded_checklists + + +def expand_checklist_action_url(course_module, checklist): + """ + Expands the action URLs for a given checklist and returns the modified version. + + The method does a copy of the input checklist and does not modify the input argument. + """ + expanded_checklist = copy.deepcopy(checklist) urlconf_map = { "ManageUsers": "manage_users", "SettingsDetails": "settings_details", @@ -109,19 +120,15 @@ def expand_checklist_action_urls(course_module): "CourseOutline": "course_index", "Checklists": "checklists", } - for checklist in checklists: - if not checklist.get('action_urls_expanded', False): - for item in checklist.get('items'): - action_url = item.get('action_url') - if action_url not in urlconf_map: - continue - urlconf_name = urlconf_map[action_url] - item['action_url'] = reverse(urlconf_name, kwargs={ - 'org': course_module.location.org, - 'course': course_module.location.course, - 'name': course_module.location.name, - }) - checklist['action_urls_expanded'] = True - modified = True + for item in expanded_checklist.get('items'): + action_url = item.get('action_url') + if action_url not in urlconf_map: + continue + urlconf_name = urlconf_map[action_url] + item['action_url'] = reverse(urlconf_name, kwargs={ + 'org': course_module.location.org, + 'course': course_module.location.course, + 'name': course_module.location.name, + }) - return checklists, modified + return expanded_checklist diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 13f0fddd48..5fa9ff3260 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -164,7 +164,7 @@ class XmlDescriptor(XModuleDescriptor): # Used for storing xml attributes between import and export, for roundtrips 'xml_attributes') - metadata_to_export_to_policy = ('discussion_topics') + metadata_to_export_to_policy = ('discussion_topics', 'checklists') @classmethod def get_map_for_field(cls, attr): diff --git a/common/test/data/toy/policies/2012_Fall.json b/common/test/data/toy/policies/2012_Fall.json index cfa5014a9b..a762123b04 100644 --- a/common/test/data/toy/policies/2012_Fall.json +++ b/common/test/data/toy/policies/2012_Fall.json @@ -38,5 +38,164 @@ }, "video/Welcome": { "display_name": "Welcome" - } + }, + "checklists": [ + { + "short_description": "Getting Started With Studio", + "items": [ + { + "action_external": false, + "short_description": "Add Course Team Members", + "action_url": "ManageUsers", + "action_text": "Edit Course Team", + "is_checked": true, + "long_description": "Grant your collaborators permission to edit your course so you can work together." + }, + { + "action_external": false, + "short_description": "Set Important Dates for Your Course", + "action_url": "SettingsDetails", + "action_text": "Edit Course Details & Schedule", + "is_checked": false, + "long_description": "Establish your course's student enrollment and launch dates on the Schedule and Details page." + }, + { + "action_external": false, + "short_description": "Draft Your Course's Grading Policy", + "action_url": "SettingsGrading", + "action_text": "Edit Grading Settings", + "is_checked": false, + "long_description": "Set up your assignment types and grading policy even if you haven't created all your assignments." + }, + { + "action_external": false, + "short_description": "Explore the Other Studio Checklists", + "action_url": "", + "action_text": "", + "is_checked": false, + "long_description": "Discover other available course authoring tools, and find help when you need it." + } + ] + }, + { + "short_description": "Draft a Rough Course Outline", + "items": [ + { + "action_external": false, + "short_description": "Create Your First Section and Subsection", + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "is_checked": false, + "long_description": "Use your course outline to build your first Section and Subsection." + }, + { + "action_external": false, + "short_description": "Set Section Release Dates", + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "is_checked": false, + "long_description": "Specify the release dates for each Section in your course. Sections become visible to students on their release dates." + }, + { + "action_external": false, + "short_description": "Designate a Subsection as Graded", + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "is_checked": false, + "long_description": "Set a Subsection to be graded as a specific assignment type. Assignments within graded Subsections count toward a student's final grade." + }, + { + "action_external": false, + "short_description": "Reordering Course Content", + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "is_checked": false, + "long_description": "Use drag and drop to reorder the content in your course." + }, + { + "action_external": false, + "short_description": "Renaming Sections", + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "is_checked": true, + "long_description": "Rename Sections by clicking the Section name from the Course Outline." + }, + { + "action_external": false, + "short_description": "Deleting Course Content", + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "is_checked": false, + "long_description": "Delete Sections, Subsections, or Units you don't need anymore. Be careful, as there is no Undo function." + }, + { + "action_external": false, + "short_description": "Add an Instructor-Only Section to Your Outline", + "action_url": "CourseOutline", + "action_text": "Edit Course Outline", + "is_checked": false, + "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." + } + ] + }, + { + "short_description": "Explore edX's Support Tools", + "items": [ + { + "action_external": true, + "short_description": "Explore the Studio Help Forum", + "action_url": "http://help.edge.edx.org/", + "action_text": "Visit Studio Help", + "is_checked": false, + "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." + }, + { + "action_external": true, + "short_description": "Enroll in edX 101", + "action_url": "https://edge.edx.org/courses/edX/edX101/How_to_Create_an_edX_Course/about", + "action_text": "Register for edX 101", + "is_checked": false, + "long_description": "Register for edX 101, edX's primer for course creation." + }, + { + "action_external": true, + "short_description": "Download the Studio Documentation", + "action_url": "http://files.edx.org/Getting_Started_with_Studio.pdf", + "action_text": "Download Documentation", + "is_checked": false, + "long_description": "Download the searchable Studio reference documentation in PDF form." + } + ] + }, + { + "short_description": "Draft Your Course About Page", + "items": [ + { + "action_external": false, + "short_description": "Draft a Course Description", + "action_url": "SettingsDetails", + "action_text": "Edit Course Schedule & Details", + "is_checked": false, + "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." + }, + { + "action_external": false, + "short_description": "Add Staff Bios", + "action_url": "SettingsDetails", + "action_text": "Edit Course Schedule & Details", + "is_checked": false, + "long_description": "Showing prospective students who their instructor will be is helpful. Include staff bios on the course About page." + }, + { + "action_external": false, + "short_description": "Add Course FAQs", + "action_url": "SettingsDetails", + "action_text": "Edit Course Schedule & Details", + "is_checked": false, + "long_description": "Include a short list of frequently asked questions about your course." + }, + { + "action_external": false, "short_description": "Add Course Prerequisites", "action_url": "SettingsDetails", "action_text": "Edit Course Schedule & Details", "is_checked": false, "long_description": "Let students know what knowledge and/or skills they should have before they enroll in your course."}] + } + ] }