diff --git a/cms/djangoapps/contentstore/tests/test_checklists.py b/cms/djangoapps/contentstore/tests/test_checklists.py new file mode 100644 index 0000000000..c20fc21832 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_checklists.py @@ -0,0 +1,93 @@ +from contentstore.utils import get_modulestore, get_url_reverse +from contentstore.tests.test_course_settings import CourseTestCase +from xmodule.modulestore.inheritance import own_metadata +from xmodule.modulestore.tests.factories import CourseFactory +from django.core.urlresolvers import reverse +import json + + +class ChecklistTestCase(CourseTestCase): + def setUp(self): + super(ChecklistTestCase, self).setUp() + self.course = CourseFactory.create(org='mitX', number='333', display_name='Checklists Course') + + def get_persisted_checklists(self): + modulestore = get_modulestore(self.course.location) + return modulestore.get_item(self.course.location).checklists + + def test_get_checklists(self): + checklists_url = get_url_reverse('Checklists', self.course) + response = self.client.get(checklists_url) + self.assertContains(response, "Getting Started With Studio") + payload = response.content + + # Now delete the checklists from the course and verify they get repopulated (for courses + # created before checklists were introduced). + self.course.checklists = None + modulestore = get_modulestore(self.course.location) + modulestore.update_metadata(self.course.location, own_metadata(self.course)) + self.assertEquals(self.get_persisted_checklists(), None) + response = self.client.get(checklists_url) + self.assertEquals(payload, response.content) + + def test_update_checklists_no_index(self): + # No checklist index, should return all of them. + update_url = reverse('checklists_updates', kwargs={ + 'org': self.course.location.org, + 'course': self.course.location.course, + 'name': self.course.location.name}) + + returned_checklists = json.loads(self.client.get(update_url).content) + self.assertListEqual(self.get_persisted_checklists(), + returned_checklists) + + def test_update_checklists_index_ignored_on_get(self): + # Checklist index ignored on get. + update_url = reverse('checklists_updates', kwargs={'org': self.course.location.org, + 'course': self.course.location.course, + 'name': self.course.location.name, + 'checklist_index': 1}) + + returned_checklists = json.loads(self.client.get(update_url).content) + self.assertListEqual(self.get_persisted_checklists(), returned_checklists) + + def test_update_checklists_post_no_index(self): + # No checklist index, will error on post. + update_url = reverse('checklists_updates', kwargs={'org': self.course.location.org, + 'course': self.course.location.course, + 'name': self.course.location.name}) + response = self.client.post(update_url) + self.assertContains(response, 'Could not save checklist', status_code=400) + + def test_update_checklists_index_out_of_range(self): + # Checklist index out of range, will error on post. + update_url = reverse('checklists_updates', kwargs={'org': self.course.location.org, + 'course': self.course.location.course, + 'name': self.course.location.name, + 'checklist_index': 100}) + response = self.client.post(update_url) + self.assertContains(response, 'Could not save checklist', status_code=400) + + def test_update_checklists_index(self): + # Checklist index out of range, will error on post. + update_url = reverse('checklists_updates', kwargs={'org': self.course.location.org, + 'course': self.course.location.course, + 'name': self.course.location.name, + 'checklist_index': 2}) + + payload = self.course.checklists[2] + self.assertFalse(payload.get('is_checked')) + payload['is_checked'] = True + + returned_checklist = json.loads(self.client.post(update_url, json.dumps(payload), "application/json").content) + self.assertTrue(returned_checklist.get('is_checked')) + self.assertEqual(self.get_persisted_checklists()[2], returned_checklist) + + def test_update_checklists_deleted_unsupported(self): + # Checklist index out of range, will error on post. + update_url = reverse('checklists_updates', kwargs={'org': self.course.location.org, + 'course': self.course.location.course, + 'name': self.course.location.name, + 'checklist_index': 100}) + response = self.client.delete(update_url) + self.assertContains(response, 'Unsupported request', status_code=400) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 55d3ac60c7..431cc50881 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -1,4 +1,4 @@ -from contentstore import utils +from contentstore import utils import mock from django.test import TestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -37,6 +37,9 @@ class UrlReverseTestCase(ModuleStoreTestCase): self.assertEquals('/mitX/666/course/URL_Reverse_Course', utils.get_url_reverse('CourseOutline', course)) + self.assertEquals('/mitX/666/checklists/URL_Reverse_Course', + utils.get_url_reverse('Checklists', course)) + def test_unknown_passes_through(self): course = CourseFactory.create(org='mitX', number='666', display_name='URL Reverse Course') self.assertEquals('foobar', diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index e9e793b2da..8cde25d185 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -177,7 +177,8 @@ def get_url_reverse(course_page_name, course_module): if CoursePageNames.ManageUsers == url_name: return reverse(url_name, kwargs={"location": ctx_loc}) - elif url_name in [CoursePageNames.SettingsDetails, CoursePageNames.SettingsGrading, CoursePageNames.CourseOutline]: + elif url_name in [CoursePageNames.SettingsDetails, CoursePageNames.SettingsGrading, + CoursePageNames.CourseOutline, CoursePageNames.Checklists]: return reverse(url_name, kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name}) else: return course_page_name @@ -188,3 +189,4 @@ class CoursePageNames: SettingsDetails = "settings_details" SettingsGrading = "settings_grading" CourseOutline = "course_index" + Checklists = "checklists" diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 796c39323f..a16775af35 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -57,7 +57,6 @@ from xmodule.modulestore.xml_importer import import_from_xml from contentstore.course_info_model import get_course_updates, \ update_course_updates, delete_course_update from cache_toolbox.core import del_cached_content -from xmodule.timeparse import stringify_time from contentstore.module_info_model import get_module_info, set_module_info from models.settings.course_details import CourseDetails, \ CourseSettingsEncoder @@ -1299,11 +1298,14 @@ def get_checklists(request, org, course, name): 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 - modulestore.update_metadata(location, own_metadata(course_module)) + copied = True - checklists = get_checklists_with_action_urls(course_module) + checklists, modified = expand_checklist_action_urls(course_module) + if copied or modified: + modulestore.update_metadata(location, own_metadata(course_module)) return render_to_response('checklists.html', { 'context_course': course_module, @@ -1330,32 +1332,42 @@ def update_checklist(request, org, course, name, checklist_index=None): real_method = get_request_method(request) if real_method == 'POST' or real_method == 'PUT': if checklist_index is not None and 0 <= int(checklist_index) < len(course_module.checklists): - modified_checklist = json.loads(request.body) - course_module.checklists[int(checklist_index)] = modified_checklist + index = int(checklist_index) + course_module.checklists[index] = json.loads(request.body) + checklists, modified = expand_checklist_action_urls(course_module) modulestore.update_metadata(location, own_metadata(course_module)) - checklists = get_checklists_with_action_urls(course_module) - return HttpResponse(json.dumps(checklists), mimetype="application/json") + return HttpResponse(json.dumps(checklists[index]), mimetype="application/json") else: return HttpResponseBadRequest("Could not save checklist state because the checklist index was out of range or unspecified.", content_type="text/plain") elif request.method == 'GET': # In the JavaScript view initialize method, we do a fetch to get all the checklists. - checklists = get_checklists_with_action_urls(course_module) + checklists, modified = expand_checklist_action_urls(course_module) + if modified: + modulestore.update_metadata(location, own_metadata(course_module)) return HttpResponse(json.dumps(checklists), mimetype="application/json") + else: + return HttpResponseBadRequest("Unsupported request.", content_type="text/plain") -def get_checklists_with_action_urls(course_module): +def expand_checklist_action_urls(course_module): """ - Gets the checklists out of the course module and expands their action urls. - Returns the checklists with modified urls (different from what is stored in the course moduule). + Gets the checklists out of the course module and expands their action urls + if they have not yet been expanded. + + Returns the checklists with modified urls, as well as a boolean + indicating whether or not the checklists were modified. """ checklists = course_module.checklists - # Expand action names to their URLs. + modified = False for checklist in checklists: - for item in checklist.get('items'): - item['action_url'] = get_url_reverse(item.get('action_url'), course_module) + if not checklist.get('action_urls_expanded', False): + for item in checklist.get('items'): + item['action_url'] = get_url_reverse(item.get('action_url'), course_module) + checklist['action_urls_expanded'] = True + modified = True - return checklists + return checklists, modified @login_required