studio - manual policy editor: converting to new page layout - WIP

cms/static/js/models/settings/advanced.js

 if (!CMS.Models['Settings']) CMS.Models.Settings = {};
 
 CMS.Models.Settings.Advanced = Backbone.Model.extend({
+    // the key for a newly added policy-- before the user has entered a key value
+    new_key : "__new_advanced_key__",
+
     defaults: {
         // the properties are whatever the user types in (in addition to whatever comes originally from the server)
     },
@@ -11,7 +14,10 @@ CMS.Models.Settings.Advanced = Backbone.Model.extend({
     validate: function (attrs) {
         var errors = {};
         for (var key in attrs) {
-            if (_.contains(this.blacklistKeys, key)) {
+            if (key === this.new_key || _.isEmpty(key)) {
+                errors[key] = "A key must be entered.";
+            }
+            else if (_.contains(this.blacklistKeys, key)) {
                 errors[key] = key + " is a reserved keyword or has another editor";
             }
         }

cms/static/js/views/settings/advanced_view.js

 if (!CMS.Views['Settings']) CMS.Views.Settings = {};
 
 CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
-    // the key for a newly added policy-- before the user has entered a key value
-    new_key : "__new_advanced_key__",
     error_saving : "error_saving",
     successful_changes: "successful_changes",
 
@@ -54,6 +52,13 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
         return this;
     },
     attachJSONEditor : function (textarea) {
+        // Since we are allowing duplicate keys at the moment, it is possible that we will try to attach
+        // JSON Editor to a value that already has one. Therefore only attach if no CodeMirror peer exists.
+        var siblings = $(textarea).siblings();
+        if (siblings.length >= 1 && $(siblings[0]).hasClass('CodeMirror')) {
+            return;
+        }
+
         var self = this;
         CodeMirror.fromTextArea(textarea, {
             mode: "application/json", lineNumbers: false, lineWrapping: false,
@@ -61,7 +66,7 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
                 self.showSaveCancelButtons();
             },
             onBlur: function (mirror) {
-                var key = $(mirror.getWrapperElement()).closest('.row').children('.key').attr('id');
+                var key = $(mirror.getWrapperElement()).closest('.field-group').children('.key').attr('id');
                 var stringValue = $.trim(mirror.getValue());
                 // update CodeMirror to show the trimmed value.
                 mirror.setValue(stringValue);
@@ -79,18 +84,20 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
                             mirror.setValue(stringValue);
                         } catch(quotedE) {
                             // TODO: validation error
-                            console.log("Error with JSON, even after converting to String.")
+                            console.log("Error with JSON, even after converting to String.");
                             console.log(quotedE);
                             JSONValue = undefined;
                         }
                     }
                     else {
                         // TODO: validation error
-                        console.log("Error with JSON, but will not convert to String.")
+                        console.log("Error with JSON, but will not convert to String.");
                         console.log(e);
                     }
                 }
                 if (JSONValue !== undefined) {
+                    // Is it OK to clear all validation errors? If we don't we get problems with errors overlaying.
+                    self.clearValidationErrors();
                     self.model.set(key, JSONValue, {validate: true});
                 }
             }
@@ -146,7 +153,7 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
         var key = $('.key', li$).attr('id');
 
         delete this.fieldToSelectorMap[key];
-        if (key !== this.new_key) {
+        if (key !== this.model.new_key) {
             this.model.deleteKeys.push(key);
             this.model.unset(key);
         }
@@ -182,9 +189,9 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
         listEle$.append(newEle);
         // disable the value entry until there's an acceptable key
         $(newEle).find('.course-advanced-policy-value').addClass('disabled');
-        this.fieldToSelectorMap[this.new_key] = this.new_key;
+        this.fieldToSelectorMap[this.model.new_key] = this.model.new_key;
         // need to re-find b/c replaceWith seems to copy rather than use the specific ele instance
-        var policyValueDivs = this.$el.find('#' + this.new_key).closest('li').find('.json');
+        var policyValueDivs = this.$el.find('#' + this.model.new_key).closest('li').find('.json');
         // only 1 but hey, let's take advantage of the context mechanism
         _.each(policyValueDivs, this.attachJSONEditor, this);
         this.toggleNewButton(false);
@@ -194,26 +201,29 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
         // old key: either the key as in the model or new_key.
         // That is, it doesn't change as the val changes until val is accepted.
         var oldKey = $(event.currentTarget).closest('.key').attr('id');
-        var newKey = $(event.currentTarget).val();
+        // TODO: validation of keys with spaces. For now at least trim strings to remove initial and
+        // trailing whitespace
+        var newKey = $.trim($(event.currentTarget).val());
         if (oldKey !== newKey) {
             // TODO: is it OK to erase other validation messages?
             this.clearValidationErrors();
 
             if (!this.validateKey(oldKey, newKey)) return;
 
-            if (this.model.has(newKey)) {
-                var error = {};
-                error[oldKey] = 'You have already defined "' + newKey + '" in the manual policy definitions.';
-                error[newKey] = "You tried to enter a duplicate of this key.";
-                this.model.trigger("error", this.model, error);
-                return false;
-            }
+//  TODO: re-enable validation
+//            if (this.model.has(newKey)) {
+//                var error = {};
+//                error[oldKey] = 'You have already defined "' + newKey + '" in the manual policy definitions.';
+//                error[newKey] = "You tried to enter a duplicate of this key.";
+//                this.model.trigger("error", this.model, error);
+//                return false;
+//            }
 
             // explicitly call validate to determine whether to proceed (relying on triggered error means putting continuation in the success
             // method which is uglier I think?)
             var newEntryModel = {};
             // set the new key's value to the old one's
-            newEntryModel[newKey] = (oldKey === this.new_key ? '' : this.model.get(oldKey));
+            newEntryModel[newKey] = (oldKey === this.model.new_key ? '' : this.model.get(oldKey));
 
             var validation = this.model.validate(newEntryModel);
             if (validation) {
@@ -231,7 +241,7 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
 
             delete this.fieldToSelectorMap[oldKey];
 
-            if (oldKey !== this.new_key) {
+            if (oldKey !== this.model.new_key) {
                 // mark the old key for deletion and delete from field maps
                 this.model.deleteKeys.push(oldKey);
                 this.model.unset(oldKey) ;
@@ -252,7 +262,9 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
             $(event.currentTarget).closest('li').replaceWith(newEle);
             // need to re-find b/c replaceWith seems to copy rather than use the specific ele instance
             var policyValueDivs = this.$el.find('#' + newKey).closest('li').find('.json');
-            // only 1 but hey, let's take advantage of the context mechanism
+
+            // Because we are not dis-allowing duplicate key definitions, we may get back more than one
+            // div. But attachJSONEditor will only attach editor if it is not already defined.
             _.each(policyValueDivs, this.attachJSONEditor, this);
             
             this.fieldToSelectorMap[newKey] = newKey;
@@ -260,13 +272,15 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
     },
     validateKey : function(oldKey, newKey) {
         // model validation can't handle malformed keys nor notice if 2 fields have same key; so, need to add that chk here
-        // TODO ensure there's no spaces or illegal chars
-        if (_.isEmpty(newKey)) {
-            var error = {};
-            error[oldKey] = "Key cannot be an empty string";
-            this.model.trigger("error", this.model, error);
-            return false;
-        }
-        else return true;
+        // TODO ensure there's no spaces or illegal chars (note some checking for spaces currently done in model's
+        // validate method.
+//        if (_.isEmpty(newKey)) {
+//            var error = {};
+//            error[oldKey] = "Key cannot be an empty string";
+//            this.model.trigger("error", this.model, error);
+//            return false;
+//        }
+//        else return true;
+        return true;
     }
 });
\ No newline at end of file

cms/static/sass/_settings.scss

@@ -14,6 +14,44 @@ body.course.settings {
     padding: $baseline ($baseline*1.5);
   }
 
+  // messages - should be synced up with global messages in the future
+  .message {
+    display: block;
+    font-size: 14px;
+  }
+
+  .message-status {
+    display: none;
+    @include border-top-radius(2px);
+    @include box-sizing(border-box);
+    border-bottom: 2px solid $yellow;
+    margin: 0 0 20px 0;
+    padding: 10px 20px;
+    font-weight: 500;
+    background: $paleYellow;
+
+    .text {
+      display: inline-block;
+    }
+
+    &.error {
+      border-color: shade($red, 50%);
+      background: tint($red, 20%);
+      color: $white;
+    }
+
+    &.confirm {
+      border-color: shade($green, 50%);
+      background: tint($green, 20%);
+      color: $white;
+    }
+
+    &.is-shown {
+      display: block;
+    }
+  }
+
+  // in form - elements
   .group-settings {
     margin: 0 0 ($baseline*2) 0;
 
@@ -45,7 +83,7 @@ body.course.settings {
 
     }
 
-    // UI hints/tips/messages
+    // in form  -UI hints/tips/messages
     .instructions {
       @include font-size(14);
       margin: 0 0 $baseline 0;
@@ -67,43 +105,6 @@ body.course.settings {
       color: $red;
     }
 
-    // messages - should be synced up with global messages in the future
-    .message {
-      display: block;
-      font-size: 14px;
-    }
-
-    .message-status {
-      display: none;
-      @include border-top-radius(2px);
-      @include box-sizing(border-box);
-      border-bottom: 2px solid $yellow;
-      margin: 0 0 20px 0;
-      padding: 10px 20px;
-      font-weight: 500;
-      background: $paleYellow;
-
-      .text {
-        display: inline-block;
-      }
-
-      &.error {
-        border-color: shade($red, 50%);
-        background: tint($red, 20%);
-        color: $white;
-      }
-
-      &.confirm {
-        border-color: shade($green, 50%);
-        background: tint($green, 20%);
-        color: $white;
-      }
-
-      &.is-shown {
-        display: block;
-      }
-    }
-
     // buttons
     .remove-item {
       @include white-button;

cms/templates/settings.html

@@ -221,6 +221,7 @@ from contentstore import utils
           <ul>
             <li class="nav-item"><a href="${reverse('contentstore.views.course_config_graders_page', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">Grading</a></li>
             <li class="nav-item"><a href="${reverse('manage_users', kwargs=dict(location=ctx_loc))}">Course Team</a></li>
+            <li class="nav-item"><a href="${reverse('course_advanced_settings', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">Advanced Settings</a></li>
           </ul>
         </nav>
       % endif

cms/templates/settings_advanced.html

 <%inherit file="base.html" />
 <%! from django.core.urlresolvers import reverse %>
-<%block name="title">Advanced</%block>
+<%block name="title">Advanced Settings</%block>
 <%block name="bodyclass">is-signedin course advanced settings</%block>
 
 <%namespace name='static' file='static_content.html'/>
@@ -53,6 +53,15 @@ editor.render();
 
     <article class="content-primary" role="main">
       <form id="settings_advanced" class="settings-advanced" method="post" action="">
+
+        <div class="message message-status confirm is-shown">
+          Your policy changes have been saved.
+        </div>
+
+        <div class="message message-status error is-shown">
+          There was an error saving your information. Please see below.
+        </div>
+
         <section class="group-settings advanced-policies">
           <header>
             <h2 class="title-2">Manual Policy Definition</h2>
@@ -89,6 +98,7 @@ editor.render();
         <nav class="nav-related">
           <ul>
             <li class="nav-item"><a href="${reverse('contentstore.views.get_course_settings', kwargs=dict(org=ctx_loc.org, course=ctx_loc.course, name=ctx_loc.name))}">Details &amp; Schedule</a></li>
+            <li class="nav-item"><a href="${reverse('contentstore.views.course_config_graders_page', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">Grading</a></li>
             <li class="nav-item"><a href="${reverse('manage_users', kwargs=dict(location=ctx_loc))}">Course Team</a></li>
           </ul>
         </nav>

cms/templates/settings_graders.html

@@ -141,6 +141,7 @@ from contentstore import utils
           <ul>
             <li class="nav-item"><a href="${reverse('contentstore.views.get_course_settings', kwargs=dict(org=ctx_loc.org, course=ctx_loc.course, name=ctx_loc.name))}">Details &amp; Schedule</a></li>
             <li class="nav-item"><a href="${reverse('manage_users', kwargs=dict(location=ctx_loc))}">Course Team</a></li>
+            <li class="nav-item"><a href="${reverse('course_advanced_settings', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">Advanced Settings</a></li>
           </ul>
         </nav>
       % endif