Commit 784115ac by Christina Roberts

Merge pull request #901 from edx/christina/dont_import_checklists

Do not store expanded action URLs.
parents 5af1fd79 cda45cd7
""" Unit tests for checklist methods in views.py. """ """ Unit tests for checklist methods in views.py. """
from contentstore.utils import get_modulestore 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.inheritance import own_metadata
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
...@@ -22,20 +23,16 @@ class ChecklistTestCase(CourseTestCase): ...@@ -22,20 +23,16 @@ class ChecklistTestCase(CourseTestCase):
def compare_checklists(self, persisted, request): def compare_checklists(self, persisted, request):
""" """
Handles url expansion as possible difference and descends into guts Handles url expansion as possible difference and descends into guts
:param persisted:
:param request:
""" """
self.assertEqual(persisted['short_description'], request['short_description']) self.assertEqual(persisted['short_description'], request['short_description'])
compare_urls = (persisted.get('action_urls_expanded') == request.get('action_urls_expanded')) expanded_checklist = expand_checklist_action_url(self.course, persisted)
pers, req = None, None for pers, req in zip(expanded_checklist['items'], request['items']):
for pers, req in zip(persisted['items'], request['items']):
self.assertEqual(pers['short_description'], req['short_description']) self.assertEqual(pers['short_description'], req['short_description'])
self.assertEqual(pers['long_description'], req['long_description']) self.assertEqual(pers['long_description'], req['long_description'])
self.assertEqual(pers['is_checked'], req['is_checked']) self.assertEqual(pers['is_checked'], req['is_checked'])
if compare_urls:
self.assertEqual(pers['action_url'], req['action_url']) self.assertEqual(pers['action_url'], req['action_url'])
self.assertEqual(pers['action_text'], req['action_text']) self.assertEqual(pers['action_text'], req['action_text'])
self.assertEqual(pers['action_external'], req['action_external']) self.assertEqual(pers['action_external'], req['action_external'])
def test_get_checklists(self): def test_get_checklists(self):
""" Tests the get checklists method. """ """ Tests the get checklists method. """
...@@ -46,6 +43,11 @@ class ChecklistTestCase(CourseTestCase): ...@@ -46,6 +43,11 @@ class ChecklistTestCase(CourseTestCase):
}) })
response = self.client.get(checklists_url) response = self.client.get(checklists_url)
self.assertContains(response, "Getting Started With Studio") 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 payload = response.content
# Now delete the checklists from the course and verify they get repopulated (for courses # Now delete the checklists from the course and verify they get repopulated (for courses
...@@ -67,7 +69,11 @@ class ChecklistTestCase(CourseTestCase): ...@@ -67,7 +69,11 @@ class ChecklistTestCase(CourseTestCase):
'name': self.course.location.name}) 'name': self.course.location.name})
returned_checklists = json.loads(self.client.get(update_url).content) 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) self.compare_checklists(pay, resp)
def test_update_checklists_index_ignored_on_get(self): def test_update_checklists_index_ignored_on_get(self):
...@@ -103,19 +109,21 @@ class ChecklistTestCase(CourseTestCase): ...@@ -103,19 +109,21 @@ class ChecklistTestCase(CourseTestCase):
update_url = reverse('checklists_updates', kwargs={'org': self.course.location.org, update_url = reverse('checklists_updates', kwargs={'org': self.course.location.org,
'course': self.course.location.course, 'course': self.course.location.course,
'name': self.course.location.name, '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.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 get_first_item(payload)['is_checked'] = True
returned_checklist = json.loads(self.client.post(update_url, json.dumps(payload), "application/json").content) 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')) self.assertTrue(get_first_item(returned_checklist).get('is_checked'))
pers = self.get_persisted_checklists() persisted_checklist = self.get_persisted_checklists()[1]
self.compare_checklists(pers[2], returned_checklist) # 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): def test_update_checklists_delete_unsupported(self):
""" Delete operation is not supported. """ """ Delete operation is not supported. """
...@@ -125,3 +133,36 @@ class ChecklistTestCase(CourseTestCase): ...@@ -125,3 +133,36 @@ class ChecklistTestCase(CourseTestCase):
'checklist_index': 100}) 'checklist_index': 100})
response = self.client.delete(update_url) response = self.client.delete(update_url)
self.assertEqual(response.status_code, 405) 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']
import json import json
import copy
from util.json_request import JsonResponse from util.json_request import JsonResponse
from django.http import HttpResponseBadRequest from django.http import HttpResponseBadRequest
...@@ -32,19 +33,16 @@ def get_checklists(request, org, course, name): ...@@ -32,19 +33,16 @@ def get_checklists(request, org, course, name):
# If course was created before checklists were introduced, copy them over # If course was created before checklists were introduced, copy them over
# from the template. # from the template.
copied = False
if not course_module.checklists: if not course_module.checklists:
course_module.checklists = CourseDescriptor.checklists.default course_module.checklists = CourseDescriptor.checklists.default
copied = True
checklists, modified = expand_checklist_action_urls(course_module)
if copied or modified:
course_module.save() course_module.save()
modulestore.update_metadata(location, own_metadata(course_module)) modulestore.update_metadata(location, own_metadata(course_module))
expanded_checklists = expand_all_action_urls(course_module)
return render_to_response('checklists.html', return render_to_response('checklists.html',
{ {
'context_course': course_module, 'context_course': course_module,
'checklists': checklists 'checklists': expanded_checklists
}) })
...@@ -68,14 +66,20 @@ def update_checklist(request, org, course, name, checklist_index=None): ...@@ -68,14 +66,20 @@ def update_checklist(request, org, course, name, checklist_index=None):
if request.method in ("POST", "PUT"): if request.method in ("POST", "PUT"):
if checklist_index is not None and 0 <= int(checklist_index) < len(course_module.checklists): if checklist_index is not None and 0 <= int(checklist_index) < len(course_module.checklists):
index = int(checklist_index) 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 # seeming noop which triggers kvs to record that the metadata is
# not default # not default
course_module.checklists = course_module.checklists course_module.checklists = course_module.checklists
checklists, _ = expand_checklist_action_urls(course_module)
course_module.save() course_module.save()
modulestore.update_metadata(location, own_metadata(course_module)) 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: else:
return HttpResponseBadRequest( return HttpResponseBadRequest(
( "Could not save checklist state because the checklist index " ( "Could not save checklist state because the checklist index "
...@@ -85,23 +89,30 @@ def update_checklist(request, org, course, name, checklist_index=None): ...@@ -85,23 +89,30 @@ def update_checklist(request, org, course, name, checklist_index=None):
elif request.method == 'GET': elif request.method == 'GET':
# In the JavaScript view initialize method, we do a fetch to get all # In the JavaScript view initialize method, we do a fetch to get all
# the checklists. # the checklists.
checklists, modified = expand_checklist_action_urls(course_module) expanded_checklists = expand_all_action_urls(course_module)
if modified: return JsonResponse(expanded_checklists)
course_module.save()
modulestore.update_metadata(location, own_metadata(course_module))
return JsonResponse(checklists) def expand_all_action_urls(course_module):
"""
Gets the checklists out of the course module and expands their action urls.
Returns a copy of the checklists with modified urls, without modifying the persisted version
of the checklists.
"""
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_urls(course_module): def expand_checklist_action_url(course_module, checklist):
""" """
Gets the checklists out of the course module and expands their action urls Expands the action URLs for a given checklist and returns the modified version.
if they have not yet been expanded.
Returns the checklists with modified urls, as well as a boolean The method does a copy of the input checklist and does not modify the input argument.
indicating whether or not the checklists were modified.
""" """
checklists = course_module.checklists expanded_checklist = copy.deepcopy(checklist)
modified = False
urlconf_map = { urlconf_map = {
"ManageUsers": "manage_users", "ManageUsers": "manage_users",
"SettingsDetails": "settings_details", "SettingsDetails": "settings_details",
...@@ -109,19 +120,15 @@ def expand_checklist_action_urls(course_module): ...@@ -109,19 +120,15 @@ def expand_checklist_action_urls(course_module):
"CourseOutline": "course_index", "CourseOutline": "course_index",
"Checklists": "checklists", "Checklists": "checklists",
} }
for checklist in checklists: for item in expanded_checklist.get('items'):
if not checklist.get('action_urls_expanded', False): action_url = item.get('action_url')
for item in checklist.get('items'): if action_url not in urlconf_map:
action_url = item.get('action_url') continue
if action_url not in urlconf_map: urlconf_name = urlconf_map[action_url]
continue item['action_url'] = reverse(urlconf_name, kwargs={
urlconf_name = urlconf_map[action_url] 'org': course_module.location.org,
item['action_url'] = reverse(urlconf_name, kwargs={ 'course': course_module.location.course,
'org': course_module.location.org, 'name': course_module.location.name,
'course': course_module.location.course, })
'name': course_module.location.name,
}) return expanded_checklist
checklist['action_urls_expanded'] = True
modified = True
return checklists, modified
...@@ -164,7 +164,7 @@ class XmlDescriptor(XModuleDescriptor): ...@@ -164,7 +164,7 @@ class XmlDescriptor(XModuleDescriptor):
# Used for storing xml attributes between import and export, for roundtrips # Used for storing xml attributes between import and export, for roundtrips
'xml_attributes') 'xml_attributes')
metadata_to_export_to_policy = ('discussion_topics') metadata_to_export_to_policy = ('discussion_topics', 'checklists')
@classmethod @classmethod
def get_map_for_field(cls, attr): def get_map_for_field(cls, attr):
......
...@@ -38,5 +38,164 @@ ...@@ -38,5 +38,164 @@
}, },
"video/Welcome": { "video/Welcome": {
"display_name": "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 &amp; 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 &amp; 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 &amp; 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 &amp; 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 &amp; Details", "is_checked": false, "long_description": "Let students know what knowledge and/or skills they should have before they enroll in your course."}]
}
]
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment