From a6faa163f3c736a9e13e634d4ccbdb9b68937b4f Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 18 Oct 2013 13:32:43 -0400 Subject: [PATCH] Convert checklists to new URL scheme. --- .../contentstore/tests/test_checklists.py | 58 ++++----- .../contentstore/views/checklist.py | 122 +++++++++--------- cms/templates/checklists.html | 4 +- cms/templates/widgets/header.html | 6 +- cms/urls.py | 4 +- 5 files changed, 90 insertions(+), 104 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_checklists.py b/cms/djangoapps/contentstore/tests/test_checklists.py index 3b26f6d7d9..d7c9e78f62 100644 --- a/cms/djangoapps/contentstore/tests/test_checklists.py +++ b/cms/djangoapps/contentstore/tests/test_checklists.py @@ -3,7 +3,8 @@ 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 +from xmodule.modulestore.django import loc_mapper + import json from .utils import CourseTestCase @@ -14,6 +15,8 @@ class ChecklistTestCase(CourseTestCase): """ Creates the test course. """ super(ChecklistTestCase, self).setUp() self.course = CourseFactory.create(org='mitX', number='333', display_name='Checklists Course') + self.location = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True) + self.checklists_url = self.location.url_reverse('checklists/', '') def get_persisted_checklists(self): """ Returns the checklists as persisted in the modulestore. """ @@ -35,13 +38,8 @@ class ChecklistTestCase(CourseTestCase): self.assertEqual(pers['action_external'], req['action_external']) def test_get_checklists(self): - """ Tests the get checklists method. """ - checklists_url = reverse("checklists", kwargs={ - 'org': self.course.location.org, - 'course': self.course.location.course, - 'name': self.course.location.name, - }) - response = self.client.get(checklists_url) + """ Tests the get checklists method and URL expansion. """ + response = self.client.get(self.checklists_url) self.assertContains(response, "Getting Started With Studio") # Verify expansion of action URL happened. self.assertContains(response, '/mitX/333/team/Checklists_Course') @@ -58,17 +56,19 @@ class ChecklistTestCase(CourseTestCase): modulestore = get_modulestore(self.course.location) modulestore.update_metadata(self.course.location, own_metadata(self.course)) self.assertEqual(self.get_persisted_checklists(), None) - response = self.client.get(checklists_url) + response = self.client.get(self.checklists_url) self.assertEqual(payload, response.content) + def test_get_checklists_html(self): + """ Tests getting the HTML template for the checklists page). """ + response = self.client.get(self.checklists_url, HTTP_ACCEPT='text/html') + self.assertContains(response, "Getting Started With Studio") + # The HTML generated will define the handler URL (for use by the Backbone model). + self.assertContains(response, self.checklists_url) + 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) + returned_checklists = json.loads(self.client.get(self.checklists_url).content) # 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() @@ -78,10 +78,7 @@ class ChecklistTestCase(CourseTestCase): 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}) + update_url = self.location.url_reverse('checklists/', '1') returned_checklists = json.loads(self.client.get(update_url).content) for pay, resp in zip(self.get_persisted_checklists(), returned_checklists): @@ -89,27 +86,19 @@ class ChecklistTestCase(CourseTestCase): 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) + response = self.client.post(self.checklists_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}) + update_url = self.location.url_reverse('checklists/', '100') + response = self.client.post(update_url) self.assertContains(response, 'Could not save checklist', status_code=400) def test_update_checklists_index(self): """ Check that an update of a particular checklist works. """ - update_url = reverse('checklists_updates', kwargs={'org': self.course.location.org, - 'course': self.course.location.course, - 'name': self.course.location.name, - 'checklist_index': 1}) + update_url = self.location.url_reverse('checklists/', '1') payload = self.course.checklists[1] self.assertFalse(get_first_item(payload).get('is_checked')) @@ -127,10 +116,7 @@ class ChecklistTestCase(CourseTestCase): def test_update_checklists_delete_unsupported(self): """ Delete operation is not supported. """ - update_url = reverse('checklists_updates', kwargs={'org': self.course.location.org, - 'course': self.course.location.course, - 'name': self.course.location.name, - 'checklist_index': 100}) + update_url = self.location.url_reverse('checklists/', '100') response = self.client.delete(update_url) self.assertEqual(response.status_code, 405) @@ -152,7 +138,7 @@ class ChecklistTestCase(CourseTestCase): 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[1], 1, 'CourseOutline', '/course/mitX.333.Checklists_Course/branch/draft/block/Checklists_Course') test_expansion(self.course.checklists[2], 0, 'http://help.edge.edx.org/', 'http://help.edge.edx.org/') diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py index 773ec087eb..173c41f99c 100644 --- a/cms/djangoapps/contentstore/views/checklist.py +++ b/cms/djangoapps/contentstore/views/checklist.py @@ -8,62 +8,63 @@ from django.views.decorators.http import require_http_methods from django.core.urlresolvers import reverse from django_future.csrf import ensure_csrf_cookie from mitxmako.shortcuts import render_to_response +from django.http import HttpResponseNotFound +from django.core.exceptions import PermissionDenied +from xmodule.modulestore.django import loc_mapper from xmodule.modulestore.inheritance import own_metadata + from ..utils import get_modulestore -from .access import get_location_and_verify_access +from .access import has_access from xmodule.course_module import CourseDescriptor +from xmodule.modulestore.locator import BlockUsageLocator -__all__ = ['get_checklists', 'update_checklist'] - - -@ensure_csrf_cookie -@login_required -def get_checklists(request, org, course, name): - """ - Send models, views, and html for displaying the course checklists. - - org, course, name: Attributes of the Location for the item to edit - """ - location = get_location_and_verify_access(request, org, course, name) - - modulestore = get_modulestore(location) - course_module = modulestore.get_item(location) - - # If course was created before checklists were introduced, copy them over - # from the template. - if not course_module.checklists: - course_module.checklists = CourseDescriptor.checklists.default - 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': expanded_checklists - }) - +__all__ = ['checklists_handler'] @require_http_methods(("GET", "POST", "PUT")) -@ensure_csrf_cookie @login_required -def update_checklist(request, org, course, name, checklist_index=None): +@ensure_csrf_cookie +def checklists_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None, checklist_index=None): """ - restful CRUD operations on course checklists. The payload is a json rep of - the modified checklist. For PUT or POST requests, the index of the - checklist being modified must be included; the returned payload will - be just that one checklist. For GET requests, the returned payload - is a json representation of the list of all checklists. + The restful handler for checklists. - org, course, name: Attributes of the Location for the item to edit + GET + html: return html page for all checklists + json: return json representing all checklists. checklist_index is not supported for GET at this time. + POST or PUT + json: updates the checked state for items within a particular checklist. checklist_index is required. """ - location = get_location_and_verify_access(request, org, course, name) - modulestore = get_modulestore(location) - course_module = modulestore.get_item(location) + location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + if not has_access(request.user, location): + raise PermissionDenied() - if request.method in ("POST", "PUT"): + old_location = loc_mapper().translate_locator_to_location(location) + + modulestore = get_modulestore(old_location) + course_module = modulestore.get_item(old_location) + + json_request = 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json') + if request.method == 'GET': + # If course was created before checklists were introduced, copy them over + # from the template. + if not course_module.checklists: + course_module.checklists = CourseDescriptor.checklists.default + course_module.save() + modulestore.update_metadata(old_location, own_metadata(course_module)) + + expanded_checklists = expand_all_action_urls(course_module) + if json_request: + return JsonResponse(expanded_checklists) + else: + handler_url = location.url_reverse('checklists/', '') + return render_to_response('checklists.html', + { + 'handler_url': handler_url, + 'checklists': expanded_checklists + }) + elif json_request: + # Can now assume POST or PUT because GET handled above. if checklist_index is not None and 0 <= int(checklist_index) < len(course_module.checklists): index = int(checklist_index) persisted_checklist = course_module.checklists[index] @@ -77,20 +78,17 @@ def update_checklist(request, org, course, name, checklist_index=None): # not default course_module.checklists = course_module.checklists course_module.save() - modulestore.update_metadata(location, own_metadata(course_module)) + modulestore.update_metadata(old_location, own_metadata(course_module)) 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 " - "was out of range or unspecified."), + "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. - expanded_checklists = expand_all_action_urls(course_module) - return JsonResponse(expanded_checklists) + else: + return HttpResponseNotFound() def expand_all_action_urls(course_module): @@ -116,19 +114,21 @@ def expand_checklist_action_url(course_module, checklist): urlconf_map = { "ManageUsers": "manage_users", "SettingsDetails": "settings_details", - "SettingsGrading": "settings_grading", - "CourseOutline": "course_index", - "Checklists": "checklists", + "SettingsGrading": "settings_grading" } + 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, - }) + if action_url == "CourseOutline": + ctx_loc = course_module.location + location = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True) + item['action_url'] = location.url_reverse('course/', '') + elif action_url in urlconf_map: + 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 expanded_checklist diff --git a/cms/templates/checklists.html b/cms/templates/checklists.html index 8a9d59512b..91ece85526 100644 --- a/cms/templates/checklists.html +++ b/cms/templates/checklists.html @@ -20,8 +20,8 @@