From 77ae9a6a783a20de3fea7f84bb24997e859e9d24 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 25 Jun 2013 16:03:27 -0400 Subject: [PATCH] PDF Textbooks: fetch/save individual textbooks Created a few RESTful API endpoints, which required creating and assigning arbitrary IDs to PDF textbooks. Changed the Backbone views to save individual models, instead of saving a whole collection. --- cms/djangoapps/contentstore/views/course.py | 135 ++++++++++++++++-- .../coffee/spec/models/textbook_spec.coffee | 3 + .../coffee/spec/views/textbook_spec.coffee | 6 +- cms/static/js/models/textbook.js | 7 + cms/static/js/views/textbook.js | 2 +- cms/urls.py | 4 + 6 files changed, 143 insertions(+), 14 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index ee9ea8375d..aa001a901d 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -2,10 +2,13 @@ Views related to operations on course objects """ import json +import random +import string from django.contrib.auth.decorators import login_required from django_future.csrf import ensure_csrf_cookie from django.conf import settings +from django.views.decorators.http import require_http_methods from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse from django.http import HttpResponse, HttpResponseBadRequest @@ -48,7 +51,8 @@ __all__ = ['course_index', 'create_new_course', 'course_info', 'course_config_advanced_page', 'course_settings_updates', 'course_grader_updates', - 'course_advanced_updates', 'textbook_index'] + 'course_advanced_updates', 'textbook_index', 'textbook_by_id', + 'create_textbook'] @login_required @@ -421,17 +425,48 @@ class TextbookValidationError(Exception): pass -def validate_textbook_json(text): +def validate_textbooks_json(text): try: - obj = json.loads(text) + textbooks = json.loads(text) except ValueError: raise TextbookValidationError("invalid JSON") - if not isinstance(obj, (list, tuple)): + if not isinstance(textbooks, (list, tuple)): raise TextbookValidationError("must be JSON list") - for textbook in obj: - if not textbook.get("tab_title"): - raise TextbookValidationError("every textbook must have a tab_title") - return obj + for textbook in textbooks: + validate_textbook_json(textbook) + # check specified IDs for uniqueness + all_ids = [textbook["id"] for textbook in textbooks if "id" in textbook] + unique_ids = set(all_ids) + if len(all_ids) > len(unique_ids): + raise TextbookValidationError("IDs must be unique") + return textbooks + + +def validate_textbook_json(textbook, used_ids=()): + if isinstance(textbook, basestring): + try: + textbook = json.loads(textbook) + except ValueError: + raise TextbookValidationError("invalid JSON") + if not isinstance(textbook, dict): + raise TextbookValidationError("must be JSON object") + if not textbook.get("tab_title"): + raise TextbookValidationError("must have tab_title") + tid = str(textbook.get("id", "")) + if tid and not tid[0].isdigit(): + raise TextbookValidationError("textbook ID must start with a digit") + return textbook + + +def assign_textbook_id(textbook, used_ids=()): + tid = Location.clean(textbook["tab_title"]) + if not tid[0].isdigit(): + # stick a random digit in front + tid = random.choice(string.digits) + tid + while tid in used_ids: + # add a random ASCII character to the end + tid = tid + random.choice(string.ascii_lowercase) + return tid @login_required @@ -451,13 +486,22 @@ def textbook_index(request, org, course, name): return JsonResponse(course_module.pdf_textbooks) elif request.method == 'POST': try: - course_module.pdf_textbooks = validate_textbook_json(request.body) + textbooks = validate_textbooks_json(request.body) except TextbookValidationError as e: return JsonResponse({"error": e.message}, status=400) + + tids = set(t["id"] for t in textbooks if "id" in t) + for textbook in textbooks: + if not "id" in textbook: + tid = assign_textbook_id(textbook, tids) + textbook["id"] = tid + tids.add(tid) + if not any(tab['type'] == 'pdf_textbooks' for tab in course_module.tabs): course_module.tabs.append({"type": "pdf_textbooks"}) + course_module.pdf_textbooks = textbooks store.update_metadata(course_module.location, own_metadata(course_module)) - return JsonResponse('', status=204) + return JsonResponse(course_module.pdf_textbooks) else: upload_asset_url = reverse('upload_asset', kwargs={ 'org': org, @@ -475,3 +519,74 @@ def textbook_index(request, org, course, name): 'upload_asset_url': upload_asset_url, 'textbook_url': textbook_url, }) + + +@login_required +@ensure_csrf_cookie +@require_http_methods(("POST",)) +def create_textbook(request, org, course, name): + location = get_location_and_verify_access(request, org, course, name) + store = get_modulestore(location) + course_module = store.get_item(location, depth=3) + + try: + textbook = validate_textbook_json(request.body) + except TextbookValidationError: + return JsonResponse({"error": e.message}, status=400) + if not textbook.get("id"): + tids = set(t["id"] for t in course_module.pdf_textbooks if "id" in t) + textbook["id"] = assign_textbook_id(textbook, tids) + course_module.pdf_textbooks.append(textbook) + store.update_metadata(course_module.location, own_metadata(course_module)) + resp = JsonResponse(textbook, status=201) + resp["Location"] = reverse("textbook_by_id", kwargs={ + 'org': org, + 'course': course, + 'name': name, + 'tid': textbook["id"], + }) + return resp + + +@login_required +@ensure_csrf_cookie +@require_http_methods(("GET", "POST", "DELETE")) +def textbook_by_id(request, org, course, name, tid): + location = get_location_and_verify_access(request, org, course, name) + store = get_modulestore(location) + course_module = store.get_item(location, depth=3) + matching_id = [tb for tb in course_module.pdf_textbooks if tb.get("id") == tid] + if matching_id: + textbook = matching_id[0] + else: + textbook = None + + if request.method == 'GET': + if not textbook: + return JsonResponse(status=404) + return JsonResponse(textbook) + elif request.method == 'POST': + try: + new_textbook = validate_textbook_json(request.body) + except TextbookValidationError: + return JsonResponse({"error": e.message}, status=400) + new_textbook["id"] = tid + if textbook: + i = course_module.pdf_textbooks.index(textbook) + new_textbooks = course_module.pdf_textbooks[0:i] + new_textbooks.append(new_textbook) + new_textbooks.extend(course_module.pdf_textbooks[i+1:]) + course_module.pdf_textbooks = new_textbooks + else: + course_module.pdf_textbooks.append(new_textbook) + store.update_metadata(course_module.location, own_metadata(course_module)) + return JsonResponse(new_textbook, status=201) + elif request.method == 'DELETE': + if not textbook: + return JsonResponse(status=404) + i = course_module.pdf_textbooks.index(textbook) + new_textbooks = course_module.pdf_textbooks[0:i] + new_textbooks.extend(course_module.pdf_textbooks[i+1:]) + course_module.pdf_textbooks = new_textbooks + store.update_metadata(course_module.location, own_metadata(course_module)) + return JsonResponse(new_textbook) diff --git a/cms/static/coffee/spec/models/textbook_spec.coffee b/cms/static/coffee/spec/models/textbook_spec.coffee index 6212c46e02..6002059119 100644 --- a/cms/static/coffee/spec/models/textbook_spec.coffee +++ b/cms/static/coffee/spec/models/textbook_spec.coffee @@ -23,6 +23,9 @@ describe "CMS.Models.Textbook", -> it "should be empty by default", -> expect(@model.isEmpty()).toBeTruthy() + it "should have a URL set", -> + expect(_.result(@model, "url")).toBeTruthy() + describe "CMS.Models.Textbook input/output", -> # replace with Backbone.Assocations.deepAttributes when diff --git a/cms/static/coffee/spec/views/textbook_spec.coffee b/cms/static/coffee/spec/views/textbook_spec.coffee index 4aa48795a3..b02e904522 100644 --- a/cms/static/coffee/spec/views/textbook_spec.coffee +++ b/cms/static/coffee/spec/views/textbook_spec.coffee @@ -76,8 +76,8 @@ describe "CMS.Views.EditTextbook", -> appendSetFixtures(sandbox({id: "page-notification"})) appendSetFixtures(sandbox({id: "page-prompt"})) @model = new CMS.Models.Textbook({name: "Life Sciences", editing: true}) + spyOn(@model, 'save') @collection = new CMS.Collections.TextbookSet() - spyOn(@collection, 'save') @collection.add(@model) @view = new CMS.Views.EditTextbook({model: @model}) spyOn(@view, 'render').andCallThrough() @@ -100,7 +100,7 @@ describe "CMS.Views.EditTextbook", -> @view.$("form").submit() expect(@model.get("name")).toEqual("starfish") expect(@model.get("chapters").at(0).get("name")).toEqual("foobar") - expect(@collection.save).toHaveBeenCalled() + expect(@model.save).toHaveBeenCalled() it "does not save on cancel", -> @model.get("chapters").add([{name: "a", asset_path: "b"}]) @@ -110,7 +110,7 @@ describe "CMS.Views.EditTextbook", -> @view.$(".action-cancel").click() expect(@model.get("name")).not.toEqual("starfish") expect(@model.get("chapters").at(0).get("name")).not.toEqual("foobar") - expect(@collection.save).not.toHaveBeenCalled() + expect(@model.save).not.toHaveBeenCalled() it "removes all empty chapters on cancel if the model has a non-empty chapter", -> chapters = @model.get("chapters") diff --git a/cms/static/js/models/textbook.js b/cms/static/js/models/textbook.js index cba1b82352..3dd5b22de6 100644 --- a/cms/static/js/models/textbook.js +++ b/cms/static/js/models/textbook.js @@ -16,6 +16,13 @@ CMS.Models.Textbook = Backbone.AssociatedModel.extend({ isEmpty: function() { return !this.get('name') && this.get('chapters').isEmpty(); }, + url: function() { + if(this.isNew()) { + return CMS.URL.TEXTBOOK + "/new"; + } else { + return CMS.URL.TEXTBOOK + "/" + this.id; + } + }, parse: function(response) { var ret = $.extend(true, {}, response); if("tab_title" in ret && !("name" in ret)) { diff --git a/cms/static/js/views/textbook.js b/cms/static/js/views/textbook.js index d12064214b..bfc0dc011f 100644 --- a/cms/static/js/views/textbook.js +++ b/cms/static/js/views/textbook.js @@ -123,7 +123,7 @@ CMS.Views.EditTextbook = Backbone.View.extend({ title: gettext("Saving…") }); var that = this; - this.model.collection.save({ + this.model.save({}, { success: function() { that.close(); }, diff --git a/cms/urls.py b/cms/urls.py index d29e355743..d58272c651 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -83,6 +83,10 @@ urlpatterns = ('', # nopep8 'contentstore.views.assets.remove_asset', name='remove_asset'), url(r'^(?P[^/]+)/(?P[^/]+)/textbooks/(?P[^/]+)$', 'contentstore.views.textbook_index', name='textbook_index'), + url(r'^(?P[^/]+)/(?P[^/]+)/textbooks/(?P[^/]+)/new$', + 'contentstore.views.create_textbook', name='create_textbook'), + url(r'^(?P[^/]+)/(?P[^/]+)/textbooks/(?P[^/]+)/(?P\d[^/]*)$', + 'contentstore.views.textbook_by_id', name='textbook_by_id'), # this is a generic method to return the data/metadata associated with a xmodule url(r'^module_info/(?P.*)$',