From fe8587566fb29389d9ace503ea2735d13f656e8f Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 2 Jul 2013 13:46:57 -0400 Subject: [PATCH] All textbook chapters must have name and asset_path Previously, for a textbook with only one chapter, we allowed that chapter to not have a title -- under the assumption that it would be shown as a monolithic textbook, instead of a chaptered textbook. However, the UI doesn't support that idea, the code was getting a bit messy, and there was no real benefit to the idea of not having to specify a chapter title. This commit removes that special case, and ensures that all textbook chapters must have a name and an asset path. --- .../coffee/spec/views/textbook_spec.coffee | 8 ++++++-- cms/static/js/models/textbook.js | 10 +--------- cms/static/js/views/textbook.js | 19 ++++--------------- 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/cms/static/coffee/spec/views/textbook_spec.coffee b/cms/static/coffee/spec/views/textbook_spec.coffee index 3edf3b4b20..ebd966239d 100644 --- a/cms/static/coffee/spec/views/textbook_spec.coffee +++ b/cms/static/coffee/spec/views/textbook_spec.coffee @@ -130,10 +130,13 @@ describe "CMS.Views.EditTextbook", -> it "should save properly", -> @view.render() @view.$("input[name=textbook-name]").val("starfish") + @view.$("input[name=chapter1-name]").val("wallflower") @view.$("input[name=chapter1-asset-path]").val("foobar") @view.$("form").submit() expect(@model.get("name")).toEqual("starfish") - expect(@model.get("chapters").at(0).get("asset_path")).toEqual("foobar") + chapter = @model.get("chapters").first() + expect(chapter.get("name")).toEqual("wallflower") + expect(chapter.get("asset_path")).toEqual("foobar") expect(@model.save).toHaveBeenCalled() it "should not save on invalid", -> @@ -151,7 +154,8 @@ describe "CMS.Views.EditTextbook", -> @view.$("input[name=chapter1-asset-path]").val("foobar.pdf") @view.$(".action-cancel").click() expect(@model.get("name")).not.toEqual("starfish") - expect(@model.get("chapters").first().get("asset_path")).not.toEqual("foobar") + chapter = @model.get("chapters").first() + expect(chapter.get("asset_path")).not.toEqual("foobar") expect(@model.save).not.toHaveBeenCalled() it "should be possible to correct validation errors", -> diff --git a/cms/static/js/models/textbook.js b/cms/static/js/models/textbook.js index 104d9ae55b..cdd86023dc 100644 --- a/cms/static/js/models/textbook.js +++ b/cms/static/js/models/textbook.js @@ -68,17 +68,9 @@ CMS.Models.Textbook = Backbone.AssociatedModel.extend({ } if (attrs.chapters.length === 0) { return { - message: "Please add at least one asset", + message: "Please add at least one chapter", attributes: {chapters: true} }; - } else if (attrs.chapters.length === 1) { - // only asset_path is required: we don't need a name - if (!attrs.chapters.first().get('asset_path')) { - return { - message: "Please add at least one asset", - attributes: {chapters: true} - }; - } } else { // validate all chapters var invalidChapters = []; diff --git a/cms/static/js/views/textbook.js b/cms/static/js/views/textbook.js index 52fcb11b76..9b6b93bf2b 100644 --- a/cms/static/js/views/textbook.js +++ b/cms/static/js/views/textbook.js @@ -143,23 +143,12 @@ CMS.Views.EditTextbook = Backbone.View.extend({ close: function() { var textbooks = this.model.collection; this.remove(); - // if the textbook has no content, remove it from the collection - if(this.model.isEmpty()) { + if(this.model.isNew()) { + // if the textbook has never been saved, remove it textbooks.remove(this.model); - } else { - // remove empty chapters from textbook - var chapters = this.model.get("chapters"); - var emptyChapters = chapters.filter(function(chapter) { - return chapter.isEmpty(); }); - if (chapters.length === emptyChapters.length) { - // make sure that there's always at least one chapter remaining - // in the chapterset, even if it's empty - emptyChapters = _.tail(emptyChapters); - } - chapters.remove(emptyChapters); - // don't forget to tell the model that it's no longer being edited - this.model.set("editing", false); } + // don't forget to tell the model that it's no longer being edited + this.model.set("editing", false); return this; } });