Merge pull request #510 from edx/feature/cdodge/allow-course-run-in-course-create

Feature/cdodge/allow course run in course create

CHANGELOG.rst

@@ -9,6 +9,9 @@ Studio: Send e-mails to new Studio users (on edge only) when their course creato
 status has changed. This will not be in use until the course creator table
 is enabled.
 
+Studio: Added improvements to Course Creation: richer error messaging, tip
+text, and fourth field for course run.
+
 LMS: Added user preferences (arbitrary user/key/value tuples, for which
 which user/key is unique) and a REST API for reading users and
 preferences. Access to the REST API is restricted by use of the

cms/djangoapps/contentstore/features/common.py

@@ -132,10 +132,12 @@ def create_studio_user(
 def fill_in_course_info(
         name='Robot Super Course',
         org='MITx',
-        num='999'):
+        num='101',
+        run='2013_Spring'):
     world.css_fill('.new-course-name', name)
     world.css_fill('.new-course-org', org)
     world.css_fill('.new-course-number', num)
+    world.css_fill('.new-course-run', run)
 
 
 def log_into_studio(

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -603,6 +603,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
             'org': 'MITx',
             'number': '999',
             'display_name': 'Robot Super Course',
+            'run': '2013_Spring'
         }
 
         module_store = modulestore('direct')
@@ -611,12 +612,12 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         resp = self.client.post(reverse('create_new_course'), course_data)
         self.assertEqual(resp.status_code, 200)
         data = parse_json(resp)
-        self.assertEqual(data['id'], 'i4x://MITx/999/course/Robot_Super_Course')
+        self.assertEqual(data['id'], 'i4x://MITx/999/course/2013_Spring')
 
         content_store = contentstore()
 
         source_location = CourseDescriptor.id_to_location('edX/toy/2012_Fall')
-        dest_location = CourseDescriptor.id_to_location('MITx/999/Robot_Super_Course')
+        dest_location = CourseDescriptor.id_to_location('MITx/999/2013_Spring')
 
         clone_course(module_store, content_store, source_location, dest_location)
 
@@ -1015,6 +1016,7 @@ class ContentStoreTest(ModuleStoreTestCase):
             'org': 'MITx',
             'number': '999',
             'display_name': 'Robot Super Course',
+            'run': '2013_Spring'
         }
 
     def tearDown(self):
@@ -1026,24 +1028,30 @@ class ContentStoreTest(ModuleStoreTestCase):
         """Test new course creation - happy path"""
         self.assert_created_course()
 
-    def assert_created_course(self):
+    def assert_created_course(self, number_suffix=None):
         """
         Checks that the course was created properly.
         """
-        resp = self.client.post(reverse('create_new_course'), self.course_data)
+        test_course_data = {}
+        test_course_data.update(self.course_data)
+        if number_suffix:
+            test_course_data['number'] = '{0}_{1}'.format(test_course_data['number'], number_suffix)
+        resp = self.client.post(reverse('create_new_course'), test_course_data)
         self.assertEqual(resp.status_code, 200)
         data = parse_json(resp)
-        self.assertEqual(data['id'], 'i4x://MITx/999/course/Robot_Super_Course')
+        self.assertNotIn('ErrMsg', data)
+        self.assertEqual(data['id'], 'i4x://MITx/{0}/course/2013_Spring'.format(test_course_data['number']))
+        return test_course_data
 
     def test_create_course_check_forum_seeding(self):
         """Test new course creation and verify forum seeding """
-        self.assert_created_course()
-        self.assertTrue(are_permissions_roles_seeded('MITx/999/Robot_Super_Course'))
+        test_course_data = self.assert_created_course(number_suffix=uuid4().hex)
+        self.assertTrue(are_permissions_roles_seeded('MITx/{0}/2013_Spring'.format(test_course_data['number'])))
 
     def test_create_course_duplicate_course(self):
         """Test new course creation - error path"""
         self.client.post(reverse('create_new_course'), self.course_data)
-        self.assert_course_creation_failed('There is already a course defined with this name.')
+        self.assert_course_creation_failed('There is already a course defined with the same organization, course number, and course run. Please change either organization or course number to be unique.')
 
     def assert_course_creation_failed(self, error_message):
         """
@@ -1058,8 +1066,9 @@ class ContentStoreTest(ModuleStoreTestCase):
         """Test new course creation - error path"""
         self.client.post(reverse('create_new_course'), self.course_data)
         self.course_data['display_name'] = 'Robot Super Course Two'
+        self.course_data['run'] = '2013_Summer'
 
-        self.assert_course_creation_failed('There is already a course defined with the same organization and course number.')
+        self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change at least one field to be unique.')
 
     def test_create_course_with_bad_organization(self):
         """Test new course creation - error path for bad organization name"""

cms/djangoapps/contentstore/views/course.py

@@ -3,6 +3,7 @@ Views related to operations on course objects
 """
 import json
 import random
+from django.utils.translation import ugettext as _
 import string  # pylint: disable=W0402
 
 from django.contrib.auth.decorators import login_required
@@ -101,12 +102,13 @@ def create_new_course(request):
     org = request.POST.get('org')
     number = request.POST.get('number')
     display_name = request.POST.get('display_name')
+    run = request.POST.get('run')
 
     try:
-        dest_location = Location('i4x', org, number, 'course', Location.clean(display_name))
+        dest_location = Location('i4x', org, number, 'course', run)
     except InvalidLocationError as error:
         return JsonResponse({
-            "ErrMsg": "Unable to create course '{name}'.\n\n{err}".format(
+            "ErrMsg": _("Unable to create course '{name}'.\n\n{err}").format(
                 name=display_name, err=error.message)})
 
     # see if the course already exists
@@ -116,12 +118,24 @@ def create_new_course(request):
     except ItemNotFoundError:
         pass
     if existing_course is not None:
-        return JsonResponse({'ErrMsg': 'There is already a course defined with this name.'})
+        return JsonResponse(
+            {
+                'ErrMsg': _('There is already a course defined with the same organization, course number, and course run. Please change either organization or course number to be unique.'),
+                'OrgErrMsg': _('Please change either the organization or course number so that it is unique.'),
+                'CourseErrMsg': _('Please change either the organization or course number so that it is unique.'),
+            }
+        )
 
     course_search_location = ['i4x', dest_location.org, dest_location.course, 'course', None]
     courses = modulestore().get_items(course_search_location)
     if len(courses) > 0:
-        return JsonResponse({'ErrMsg': 'There is already a course defined with the same organization and course number.'})
+        return JsonResponse(
+            {
+                'ErrMsg': _('There is already a course defined with the same organization and course number. Please change at least one field to be unique.'),
+                'OrgErrMsg': _('Please change either the organization or course number so that it is unique.'),
+                'CourseErrMsg': _('Please change either the organization or course number so that it is unique.'),
+            }
+        )
 
     # instantiate the CourseDescriptor and then persist it
     # note: no system to pass

cms/static/js/base.js

@@ -597,11 +597,9 @@ function cancelNewSection(e) {
 
 function addNewCourse(e) {
     e.preventDefault();
-    $('.new-course-button').addClass('disabled');
-    $(e.target).addClass('disabled');
-    var $newCourse = $($('#new-course-template').html());
+    $('.new-course-button').addClass('is-disabled');
+    var $newCourse = $('.wrapper-create-course').addClass('is-shown');
     var $cancelButton = $newCourse.find('.new-course-cancel');
-    $('.courses').prepend($newCourse);
     $newCourse.find('.new-course-name').focus().select();
     $newCourse.find('form').bind('submit', saveNewCourse);
     $cancelButton.bind('click', cancelNewCourse);
@@ -613,41 +611,97 @@ function addNewCourse(e) {
 function saveNewCourse(e) {
     e.preventDefault();
 
-    var $newCourse = $(this).closest('.new-course');
-    var org = $newCourse.find('.new-course-org').val();
-    var number = $newCourse.find('.new-course-number').val();
-    var display_name = $newCourse.find('.new-course-name').val();
+    var $newCourseForm = $(this).closest('#create-course-form');
+    var display_name = $newCourseForm.find('.new-course-name').val();
+    var org = $newCourseForm.find('.new-course-org').val();
+    var number = $newCourseForm.find('.new-course-number').val();
+    var run = $newCourseForm.find('.new-course-run').val();
 
-    if (org == '' || number == '' || display_name == '') {
-        alert(gettext('You must specify all fields in order to create a new course.'));
-        return;
+    var required_field_text = gettext('Required field');
+
+    var display_name_errMsg = (display_name === '') ? required_field_text : null;
+    var org_errMsg = (org === '') ? required_field_text : null;
+    var number_errMsg = (number === '') ? required_field_text : null;
+    var run_errMsg = (run === '') ? required_field_text : null;
+
+    var bInErr = (display_name_errMsg || org_errMsg || number_errMsg || run_errMsg);
+
+    // check for suitable encoding
+    if (!bInErr) {
+        var encoding_errMsg = gettext('Please do not use any spaces or special characters in this field.');
+
+        if (encodeURIComponent(org) != org)
+            org_errMsg = encoding_errMsg;
+        if (encodeURIComponent(number) != number)
+            number_errMsg = encoding_errMsg;
+        if (encodeURIComponent(run) != run)
+            run_errMsg = encoding_errMsg;
+
+        bInErr = (org_errMsg || number_errMsg || run_errMsg);
     }
 
+    var header_err_msg = (bInErr) ? gettext('Please correct the fields below.') : null;
+
+    var setNewCourseErrMsgs = function(header_err_msg, display_name_errMsg, org_errMsg, number_errMsg, run_errMsg) {
+        if (header_err_msg) {
+            $('.wrapper-create-course').addClass('has-errors');
+            $('.wrap-error').addClass('is-shown');
+            $('#course_creation_error').html('<p>' + header_err_msg + '</p>');
+        } else {
+            $('.wrap-error').removeClass('is-shown');
+            $('#course_creation_error').html('');
+        }
+
+        var setNewCourseFieldInErr = function(el, msg) {
+            el.children('.tip-error').remove();
+            if (msg !== null && msg !== '') {
+                el.addClass('error');
+                el.append('<span class="tip tip-error">' + msg + '</span>');
+            } else {
+                el.removeClass('error');
+            }
+        };
+
+        setNewCourseFieldInErr($('#field-course-name'), display_name_errMsg);
+        setNewCourseFieldInErr($('#field-organization'), org_errMsg);
+        setNewCourseFieldInErr($('#field-course-number'), number_errMsg);
+        setNewCourseFieldInErr($('#field-course-run'), run_errMsg);
+    };
+
+    setNewCourseErrMsgs(header_err_msg, display_name_errMsg, org_errMsg, number_errMsg, run_errMsg);
+
+    if (bInErr)
+        return;
+
     analytics.track('Created a Course', {
         'org': org,
         'number': number,
-        'display_name': display_name
+        'display_name': display_name,
+        'run': run
     });
 
     $.post('/create_new_course', {
-        'org': org,
-        'number': number,
-        'display_name': display_name
-    },
-
-    function(data) {
-        if (data.id != undefined) {
-            window.location = '/' + data.id.replace(/.*:\/\//, '');
-        } else if (data.ErrMsg != undefined) {
-            alert(data.ErrMsg);
+            'org': org,
+            'number': number,
+            'display_name': display_name,
+            'run': run
+        },
+        function(data) {
+            if (data.id !== undefined) {
+                window.location = '/' + data.id.replace(/.*:\/\//, '');
+            } else if (data.ErrMsg !== undefined) {
+                var orgErrMsg = (data.OrgErrMsg !== undefined) ? data.OrgErrMsg : null;
+                var courseErrMsg = (data.CourseErrMsg !== undefined) ? data.CourseErrMsg : null;
+                setNewCourseErrMsgs(data.ErrMsg, null, orgErrMsg, courseErrMsg, null);
+            }
         }
-    });
+    );
 }
 
 function cancelNewCourse(e) {
     e.preventDefault();
-    $('.new-course-button').removeClass('disabled');
-    $(this).parents('section.new-course').remove();
+    $('.new-course-button').removeClass('is-disabled');
+    $('.wrapper-create-course').removeClass('is-shown');
 }
 
 function addNewSubsection(e) {

cms/static/sass/_shame.scss

@@ -23,6 +23,13 @@ body.dashboard {
 }
 
 
+// yes we have no boldness today - need to fix the resets
+body strong,
+body b {
+  font-weight: 700;
+}
+
+// known things to do (paint the fence, sand the floor, wax on/off)
 // ====================
 
 

cms/static/sass/elements/_forms.scss

@@ -93,7 +93,6 @@ form {
   }
 }
 
-
 // ELEM: form wrapper
 .wrapper-create-element {
   height: 0;
@@ -117,10 +116,6 @@ form {
 // form styling for creating a new content item (course, user, textbook)
 form[class^="create-"] {
   @extend .ui-window;
-  @include box-sizing(border-box);
-  border-radius: 2px;
-  width: 100%;
-  background: $white;
 
   .title {
     @extend .t-title4;
@@ -171,8 +166,8 @@ form[class^="create-"] {
 
 
       input, textarea {
-        @include transition(all $tmg-f2 ease-in-out 0s);
         @extend .t-copy-base;
+        @include transition(all $tmg-f2 ease-in-out 0s);
         height: 100%;
         width: 100%;
         padding: ($baseline/2);
@@ -185,6 +180,10 @@ form[class^="create-"] {
           width: 25%;
         }
 
+        /*@include placeholder {
+          color: $gray-l3;
+        }*/
+
         &:focus {
 
           + .tip {
@@ -295,32 +294,26 @@ form[class^="create-"] {
     padding: ($baseline*0.75) ($baseline*1.5);
     background: $gray-l6;
 
-    .action-primary {
-      @include blue-button;
-      @extend .t-action2;
-      @include transition(all .15s);
+    .action {
+      @include transition(all $tmg-f2 linear 0s);
       display: inline-block;
       padding: ($baseline/5) $baseline;
       font-weight: 600;
       text-transform: uppercase;
     }
 
+    .action-primary {
+      @include blue-button;
+      @extend .t-action2;
+    }
+
     .action-secondary {
       @include grey-button;
       @extend .t-action2;
-      @include transition(all .15s);
-      display: inline-block;
-      padding: ($baseline/5) $baseline;
-      font-weight: 600;
-      text-transform: uppercase;
     }
   }
 }
 
-
-
-
-
 // ====================
 
 // forms - grandfathered

cms/static/sass/views/_dashboard.scss

@@ -358,22 +358,30 @@ body.dashboard {
 		}
 	}
 
-	.new-course {
-		@include clearfix();
-		padding: ($baseline*0.75) ($baseline*1.25);
-		margin-top: $baseline;
-		border-radius: 3px;
-		border: 1px solid $gray;
-		background: $white;
-		box-shadow: 0 1px 2px rgba(0, 0, 0, .1);
 
-		.title {
-			@extend .t-title4;
-			font-weight: 600;
-			margin-bottom: ($baseline/2);
-			border-bottom: 1px solid $gray-l3;
-			padding-bottom: ($baseline/2);
-		}
+  // ELEM: new user form
+  .wrapper-create-course {
+
+    // CASE: when form is animating
+    &.animate {
+
+      // STATE: shown
+      &.is-shown {
+        height: ($baseline*26);
+
+        // STATE: errors
+        &.has-errors {
+          height: ($baseline*33);
+        }
+      }
+    }
+  }
+
+  // ====================
+
+  // course listings
+
+	.create-course {
 
 		.row {
 			@include clearfix();
@@ -389,10 +397,6 @@ body.dashboard {
 			margin-right: 4%;
 		}
 
-		.course-info {
-			width: 600px;
-		}
-
 		label {
 			@extend .t-title7;
 			display: block;
@@ -401,7 +405,8 @@ body.dashboard {
 
 		.new-course-org,
 		.new-course-number,
-		.new-course-name {
+		.new-course-name,
+		.new-course-run {
 			width: 100%;
 		}
 
@@ -421,5 +426,25 @@ body.dashboard {
 		.item-details {
 			padding-bottom: 0;
 		}
+
+	    .wrap-error {
+	      @include transition(all $tmg-f2 ease 0s);
+	      height: 0;
+	      overflow: hidden;
+	      opacity: 0;
+	    }
+
+	    .wrap-error.is-shown {
+	      height: 65px;
+	      opacity: 1;
+	    }
+
+	    .message-status {
+	      display: block;
+	      margin-bottom: 0;
+	      padding: ($baseline*.5) ($baseline*1.5) 8px ($baseline*1.5);
+	      font-weight: bold;
+	    }
+
 	}
 }

cms/templates/index.html

@@ -36,36 +36,6 @@
 </script>
 </%block>
 
-<%block name="header_extras">
-  <script type="text/template" id="new-course-template">
-    <section class="new-course">
-      <h3 class="title">${_("Create a New Course:")}</h3>
-      <div class="item-details">
-        <form class="course-info">
-          <div class="row">
-	    <label>${_("Course Name")}</label>
-            <input type="text" class="new-course-name" />
-          </div>
-          <div class="row">
-            <div class="column">
-	      <label>${_("Organization")}</label>
-              <input type="text" class="new-course-org" />
-            </div>
-            <div class="column">
-	      <label>${_("Course Number")}</label>
-              <input type="text" class="new-course-number" />
-            </div>
-          </div>
-          <div class="row">
-            <input type="submit" value="${_('Save')}" class="new-course-save"/>
-            <input type="button" value="${_('Cancel')}" class="new-course-cancel" />
-          </div>
-        </form>
-      </div>
-    </section>
-  </script>
-</%block>
-
 <%block name="content">
 <div class="wrapper-mast wrapper">
   <header class="mast has-actions">
@@ -109,6 +79,57 @@
         %endif
       </div>
 
+      % if course_creator_status=='granted':
+      <div class="wrapper-create-element wrapper-create-course">
+        <form class="create-course course-info" id="create-course-form" name="create-course-form">
+          <div class="wrap-error">
+            <div id="course_creation_error" name="course_creation_error" class="message message-status message-status error" role="alert">
+            <p>${_("Please correct the highlighted fields below.")}</p>
+            </div>
+          </div>
+
+          <div class="wrapper-form">
+            <h3 class="title">${_("Create a New Course")}</h3>
+
+            <fieldset>
+              <legend class="sr">${_("Required Information to Create a New Course")}</legend>
+
+              <ol class="list-input">
+                <li class="field field-inline text required" id="field-course-name">
+                  <label for="new-course-name">${_("Course Name")}</label>
+                  <input class="new-course-name" id="new-course-name" type="text" name="new-course-name" aria-required="true" placeholder="${_('e.g. Introduction to Computer Science')}" />
+                  <span class="tip tip-stacked">${_("The public display name for your course.")}</span>
+                </li>
+                <li class="field field-inline text required" id="field-organization">
+                  <label for="new-course-org">${_("Organization")}</label>
+                  <input class="new-course-org" id="new-course-org" type="text" name="new-course-org" aria-required="true" placeholder="${_('e.g. MITX or IMF')}" />
+                  <span class="tip tip-stacked">${_("The name of the organization sponsoring the course")}  - <strong>${_("Note: No spaces or special characters are allowed. This cannot be changed.")}</strong></span>
+                </li>
+
+                <li class="field field-inline text required" id="field-course-number">
+                  <label for="new-course-number">${_("Course Number")}</label>
+                  <input class="new-course-number" id="new-course-number" type="text" name="new-course-number" aria-required="true" placeholder="${_('e.g. CS101')}" />
+                  <span class="tip tip-stacked">${_("The unique number that identifies your course within your organization")} - <strong>${_("Note: No spaces or special characters are allowed. This cannot be changed.")}</strong></span>
+                </li>
+
+                <li class="field field-inline text required" id="field-course-run">
+                  <label for="new-course-run">${_("Course Run")}</label>
+                  <input class="new-course-run" id="new-course-run" type="text" name="new-course-run" aria-required="true"placeholder="${_('e.g. 2013_Spring')}" />
+                  <span class="tip tip-stacked">${_("The term in which your course will run")} - <strong>${_("Note: No spaces or special characters are allowed. This cannot be changed.")}</strong></span>
+                </li>
+              </ol>
+
+            </fieldset>
+          </div>
+
+          <div class="actions">
+            <input type="submit" value="${_('Save')}" class="action action-primary new-course-save" />
+            <input type="button" value="${_('Cancel')}" class="action action-secondary action-cancel new-course-cancel" />
+          </div>
+        </form>
+      </div>
+      % endif
+
       %if len(courses) > 0:
       <div class="courses">
         <ul class="list-courses">

cms/templates/manage_users.html

@@ -38,6 +38,7 @@
 
             <fieldset class="form-fields">
               <legend class="sr">${_("New Team Member Information")}</legend>
+
                 <ol class="list-input">
                   <li class="field text required create-user-email">
                     <label for="user-email-input">${_("User's Email Address")}</label>
@@ -47,6 +48,7 @@
                 </ol>
             </fieldset>
           </div>
+
           <div class="actions">
             <button class="action action-primary" type="submit">${_("Add User")}</button>
             <button class="action action-secondary action-cancel">${_("Cancel")}</button>