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

@@ -604,6 +604,7 @@ def test_clone_course(self):
             'org': 'MITx',
             'number': '999',
             'display_name': 'Robot Super Course',
+            'run': '2013_Spring'
         }
 
         module_store = modulestore('direct')
@@ -612,12 +613,12 @@ def test_clone_course(self):
         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)
 
@@ -954,6 +955,7 @@ def setUp(self):
             'org': 'MITx',
             'number': '999',
             'display_name': 'Robot Super Course',
+            'run': '2013_Spring'
         }
 
     def tearDown(self):
@@ -965,24 +967,30 @@ def test_create_course(self):
         """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):
         """
@@ -997,8 +1005,9 @@ def test_create_course_duplicate_number(self):
         """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 @@
 """
 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