From 9f8f64cffe7ac2ac918a6f29dfac8b60ee84db05 Mon Sep 17 00:00:00 2001 From: Mathew Peterson Date: Thu, 31 Jul 2014 19:56:02 +0000 Subject: [PATCH 01/42] Course Reruns UI Studio: adding course re-run-centric static template rendering * initial HTML for dashboard states * initial HTML for new course re-run view/form * initial HTML placeholder for outline alert UI Conflicts: cms/templates/index.html Studio: adding styling for course re-run-centric views * adding new view/page mast-wizard type * refactoring create course/element form styling * adding course re-run view specific styling * adding courses processing styling (w/ alerts and status) Course rerun server-side updates: support display_name and DuplicateCourseError. Studio: further design revisions and tweaks from feedback * removing new window attribute from re-run control * removing links from processing courses * revising look/feel of dismiss action on dashboard + alert * correcting font-weight of dashboard processing title * adding extra space to course rerun action on dashboard * re-wording secondary cancel action on rerun view Conflicts: cms/templates/index.html Added interation on unsucceeded courses in dashboard Studio: removing 'rel=external' property from course re-run actions Studio: removing hover styles for processing courses Fixed value bug in split and set course listing to display run moved task.py for rerun --- .../contentstore/{views => }/tasks.py | 3 +- .../contentstore/tests/test_contentstore.py | 6 +- .../contentstore/tests/test_course_listing.py | 4 +- cms/djangoapps/contentstore/views/course.py | 63 ++- cms/envs/devstack.py | 7 + cms/static/js/index.js | 12 + cms/static/js/views/course_rerun.js | 161 ++++++++ cms/static/js/views/overview.js | 13 + cms/static/sass/_base.scss | 276 +++++++++++++ cms/static/sass/elements/_controls.scss | 32 +- cms/static/sass/elements/_forms.scss | 51 +-- .../sass/elements/_system-feedback.scss | 38 +- cms/static/sass/style-app-extend1.scss | 1 + cms/static/sass/views/_course-create.scss | 116 ++++++ cms/static/sass/views/_dashboard.scss | 359 ++++++++++++----- cms/templates/base.html | 4 +- cms/templates/course-create-rerun.html | 173 ++++++++ cms/templates/course_outline.html | 25 ++ cms/templates/index.html | 112 +++++- cms/templates/manage_users.html | 4 +- .../ux/reference/course-create-rerun.html | 368 ++++++++++++++++++ cms/urls.py | 1 + .../course_action_state/managers.py | 3 +- .../migrations/0002_add_rerun_display_name.py | 76 ++++ .../djangoapps/course_action_state/models.py | 3 + .../tests/test_rerun_manager.py | 21 +- .../xmodule/modulestore/split_mongo/split.py | 2 +- 27 files changed, 1771 insertions(+), 163 deletions(-) rename cms/djangoapps/contentstore/{views => }/tasks.py (99%) create mode 100644 cms/static/js/views/course_rerun.js create mode 100644 cms/static/sass/views/_course-create.scss create mode 100644 cms/templates/course-create-rerun.html create mode 100644 cms/templates/ux/reference/course-create-rerun.html create mode 100644 common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py diff --git a/cms/djangoapps/contentstore/views/tasks.py b/cms/djangoapps/contentstore/tasks.py similarity index 99% rename from cms/djangoapps/contentstore/views/tasks.py rename to cms/djangoapps/contentstore/tasks.py index d0e18e62b8..8e309f2bad 100644 --- a/cms/djangoapps/contentstore/views/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -5,6 +5,7 @@ This file contains celery tasks for contentstore views from celery.task import task from django.contrib.auth.models import User from xmodule.modulestore.django import modulestore + from xmodule.modulestore.exceptions import DuplicateCourseError, ItemNotFoundError from course_action_state.models import CourseRerunState from contentstore.utils import initialize_permissions @@ -32,13 +33,11 @@ def rerun_course(source_course_key_string, destination_course_key_string, user_i # update state: Succeeded CourseRerunState.objects.succeeded(course_key=destination_course_key) - return "succeeded" except DuplicateCourseError as exc: # do NOT delete the original course, only update the status CourseRerunState.objects.failed(course_key=destination_course_key, exception=exc) - return "duplicate course" # catch all exceptions so we can update the state and properly cleanup the course. diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 79f778ae9b..911d1a31bb 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1580,9 +1580,12 @@ class RerunCourseTest(ContentStoreTestCase): json_resp = parse_json(response) self.assertNotIn('ErrMsg', json_resp) destination_course_key = CourseKey.from_string(json_resp['destination_course_key']) - return destination_course_key + def create_course_listing_html(self, course_key): + """Creates html fragment that is created for the given course_key in the course listing section""" + return '' + data.ErrMsg + '

'); + $('.rerun-course-save').addClass('is-disabled').removeClass('is-processing').html(gettext('Create Re-run')); + $('.action-cancel').removeClass('is-hidden'); + } + } + ); + // Go into creating re-run state + $('.rerun-course-save').addClass('is-disabled').addClass('is-processing').html( + '' + gettext('Processing Re-run Request') + ); + $('.action-cancel').addClass('is-hidden'); + }; + + var cancelRerunCourse = function (e) { + e.preventDefault(); + // Clear out existing fields and errors + $('.rerun-course-run').val(''); + $('#course_rerun_error').html(''); + $('wrapper-error').removeClass('is-shown').addClass('is-hidden'); + $('.rerun-course-save').off('click'); + window.location.href = '/course/' + }; + + var validateRequiredField = function (msg) { + return msg.length === 0 ? gettext('Required field.') : ''; + }; + + var setNewCourseFieldInErr = function (el, msg) { + if(msg) { + el.addClass('error'); + el.children('span.tip-error').addClass('is-shown').removeClass('is-hidden').text(msg); + $('.rerun-course-save').addClass('is-disabled'); + } + else { + el.removeClass('error'); + el.children('span.tip-error').addClass('is-hidden').removeClass('is-shown'); + // One "error" div is always present, but hidden or shown + if($('.error').length === 1) { + $('.rerun-course-save').removeClass('is-disabled'); + } + } + }; + + domReady(function () { + var $cancelButton = $('.rerun-course-cancel'); + var $courseRun = $('.rerun-course-run'); + $courseRun.focus().select(); + $('.rerun-course-save').on('click', saveRerunCourse); + $cancelButton.bind('click', cancelRerunCourse); + CancelOnEscape($cancelButton); + $('.cancel-button').bind('click', cancelRerunCourse); + + // Check that a course (org, number, run) doesn't use any special characters + var validateCourseItemEncoding = function (item) { + var required = validateRequiredField(item); + if (required) { + return required; + } + if ($('.allow-unicode-course-id').val() === 'True'){ + if (/\s/g.test(item)) { + return gettext('Please do not use any spaces in this field.'); + } + } + else{ + if (item !== encodeURIComponent(item)) { + return gettext('Please do not use any spaces or special characters in this field.'); + } + } + return ''; + }; + + // Ensure that org/course_num/run < 65 chars. + var validateTotalCourseItemsLength = function () { + var totalLength = _.reduce( + ['.rerun-course-org', '.rerun-course-number', '.rerun-course-run'], + function (sum, ele) { + return sum + $(ele).val().length; + }, 0 + ); + if (totalLength > 65) { + $('.wrap-error').addClass('is-shown'); + $('#course_creation_error').html('

' + gettext('The combined length of the organization, course number, and course run fields cannot be more than 65 characters.') + '

'); + $('.rerun-course-save').addClass('is-disabled'); + } + else { + $('.wrap-error').removeClass('is-shown'); + } + }; + + // Handle validation asynchronously + _.each( + ['.rerun-course-org', '.rerun-course-number', '.rerun-course-run'], + function (ele) { + var $ele = $(ele); + $ele.on('keyup', function (event) { + // Don't bother showing "required field" error when + // the user tabs into a new field; this is distracting + // and unnecessary + if (event.keyCode === 9) { + return; + } + var error = validateCourseItemEncoding($ele.val()); + setNewCourseFieldInErr($ele.parent(), error); + validateTotalCourseItemsLength(); + }); + } + ); + var $name = $('.rerun-course-name'); + $name.on('keyup', function () { + var error = validateRequiredField($name.val()); + setNewCourseFieldInErr($name.parent(), error); + validateTotalCourseItemsLength(); + }); + }); + }); \ No newline at end of file diff --git a/cms/static/js/views/overview.js b/cms/static/js/views/overview.js index f33f0c908f..8ceaa40681 100644 --- a/cms/static/js/views/overview.js +++ b/cms/static/js/views/overview.js @@ -5,6 +5,17 @@ define(["domReady", "jquery", "jquery.ui", "underscore", "gettext", "js/views/fe var modalSelector = '.edit-section-publish-settings'; + var dismissNotification = function (e) { + e.preventDefault(); + $.ajax({ + url: $('.dismiss-button').data('dismiss-link'), + type: 'GET', + success: function(result) { + $('.wrapper-alert-announcement').remove() + } + }); + }; + var toggleSections = function(e) { e.preventDefault(); @@ -222,6 +233,8 @@ define(["domReady", "jquery", "jquery.ui", "underscore", "gettext", "js/views/fe $('.toggle-button-sections').bind('click', toggleSections); $('.expand-collapse').bind('click', toggleSubmodules); + $('.dismiss-button').bind('click', dismissNotification); + var $body = $('body'); $body.on('click', '.section-published-date .edit-release-date', editSectionPublishDate); $body.on('click', '.edit-section-publish-settings .action-save', saveSetSectionScheduleDate); diff --git a/cms/static/sass/_base.scss b/cms/static/sass/_base.scss index b1b57f5ff0..547d9b2e39 100644 --- a/cms/static/sass/_base.scss +++ b/cms/static/sass/_base.scss @@ -240,6 +240,282 @@ p, ul, ol, dl { } } +// ==================== + +// layout - basic +.wrapper-view { + +} + +// ==================== + +// layout - basic page header +.wrapper-mast { + margin: ($baseline*1.5) 0 0 0; + padding: 0 $baseline; + position: relative; + + .mast, .metadata { + @include clearfix(); + position: relative; + max-width: $fg-max-width; + min-width: $fg-min-width; + width: flex-grid(12); + margin: 0 auto $baseline auto; + color: $gray-d2; + } + + .mast { + border-bottom: 1px solid $gray-l4; + padding-bottom: ($baseline/2); + + // layout with actions + .page-header { + width: flex-grid(12); + } + + // layout with actions + &.has-actions { + @include clearfix(); + + .page-header { + float: left; + width: flex-grid(6,12); + margin-right: flex-gutter(); + } + + .nav-actions { + position: relative; + bottom: -($baseline*0.75); + float: right; + width: flex-grid(6,12); + text-align: right; + + .nav-item { + display: inline-block; + vertical-align: top; + margin-right: ($baseline/2); + + &:last-child { + margin-right: 0; + } + } + + // buttons + .button { + padding: ($baseline/4) ($baseline/2) ($baseline/3) ($baseline/2); + } + + .new-button { + + } + + .view-button { + + } + } + } + + // layout with actions + &.has-subtitle { + + .nav-actions { + bottom: -($baseline*1.5); + } + } + + // layout with navigation + &.has-navigation { + + .nav-actions { + bottom: -($baseline*1.5); + } + + .navigation-link { + @extend %cont-truncated; + display: inline-block; + vertical-align: bottom; // correct for extra padding in FF + max-width: 250px; + + &.navigation-current { + @extend %ui-disabled; + color: $gray; + max-width: 250px; + + &:before { + color: $gray; + } + } + } + + .navigation-link:before { + content: " / "; + margin: ($baseline/4); + color: $gray; + + &:hover { + color: $gray; + } + } + + .navigation .navigation-link:first-child:before { + content: ""; + margin: 0; + } + } + } + + // CASE: wizard-based mast + .mast-wizard { + + .page-header-sub { + @extend %t-title4; + color: $gray; + font-weight: 300; + } + + .page-header-super { + @extend %t-title4; + float: left; + width: flex-grid(12,12); + margin-top: ($baseline/2); + border-top: 1px solid $gray-l4; + padding-top: ($baseline/2); + font-weight: 600; + } + } + + // page metadata/action bar + .metadata { + + } +} + +// layout - basic page content +.wrapper-content { + margin: 0; + padding: 0 $baseline; + position: relative; +} + +.content { + @include clearfix(); + @extend %t-copy-base; + max-width: $fg-max-width; + min-width: $fg-min-width; + width: flex-grid(12); + margin: 0 auto; + color: $gray-d2; + + header { + position: relative; + margin-bottom: $baseline; + border-bottom: 1px solid $gray-l4; + padding-bottom: ($baseline/2); + + .title-sub { + @extend %t-copy-sub1; + display: block; + margin: 0; + color: $gray-l2; + } + + .title-1 { + @extend %t-title3; + margin: 0; + padding: 0; + font-weight: 600; + color: $gray-d3; + } + } +} + +.content-primary, .content-supplementary { + @include box-sizing(border-box); +} + +// layout - primary content +.content-primary { + + .title-1 { + @extend %t-title3; + } + + .title-2 { + @extend %t-title4; + margin: 0 0 ($baseline/2) 0; + } + + .title-3 { + @extend %t-title6; + margin: 0 0 ($baseline/2) 0; + } + + header { + @include clearfix(); + + .title-2 { + width: flex-grid(5, 12); + margin: 0 flex-gutter() 0 0; + float: left; + } + + .tip { + @extend %t-copy-sub2; + width: flex-grid(7, 12); + float: right; + margin-top: ($baseline/2); + text-align: right; + color: $gray-l2; + } + } +} + +// layout - supplemental content +.content-supplementary { + + > section { + margin: 0 0 $baseline 0; + } +} + +// ==================== + +// layout - grandfathered +.main-wrapper { + position: relative; + margin: 0 ($baseline*2); +} + +.inner-wrapper { + @include clearfix(); + position: relative; + max-width: 1280px; + margin: auto; + + > article { + clear: both; + } +} + +.main-column { + clear: both; + float: left; + width: 70%; +} + +.sidebar { + float: right; + width: 28%; +} + +.left { + float: left; +} + +.right { + float: right; +} // ==================== diff --git a/cms/static/sass/elements/_controls.scss b/cms/static/sass/elements/_controls.scss index 16434c5410..f5c9eb7434 100644 --- a/cms/static/sass/elements/_controls.scss +++ b/cms/static/sass/elements/_controls.scss @@ -133,6 +133,27 @@ } } +// white secondary button +%btn-secondary-white { + @extend %ui-btn-secondary; + border-color: $white-t2; + color: $white-t3; + + &:hover, &:active { + border-color: $white; + color: $white; + } + + &.current, &.active { + background: $gray-d2; + color: $gray-l5; + + &:hover, &:active { + background: $gray-d2; + } + } +} + // green secondary button %btn-secondary-green { @extend %ui-btn-secondary; @@ -213,17 +234,6 @@ // ==================== -// calls-to-action - -// ==================== - -// specific buttons - view live -%view-live-button { - @extend %t-action4; -} - -// ==================== - // UI: element actions list %actions-list { diff --git a/cms/static/sass/elements/_forms.scss b/cms/static/sass/elements/_forms.scss index ea0e144ec8..08bca8770e 100644 --- a/cms/static/sass/elements/_forms.scss +++ b/cms/static/sass/elements/_forms.scss @@ -137,28 +137,10 @@ form { } } -// ELEM: form wrapper -.wrapper-create-element { - height: 0; - margin-bottom: $baseline; - opacity: 0.0; - pointer-events: none; - overflow: hidden; - - &.animate { - @include transition(opacity $tmg-f1 ease-in-out 0s, height $tmg-f1 ease-in-out 0s); - } - - &.is-shown { - height: auto; // define a specific height for the animating version of this UI to work properly - opacity: 1.0; - pointer-events: auto; - } -} - // ELEM: form // form styling for creating a new content item (course, user, textbook) -form[class^="create-"] { +// TODO: refactor this into a placeholder to extend. +.form-create { @extend %ui-window; .title { @@ -253,12 +235,19 @@ form[class^="create-"] { .tip { @extend %t-copy-sub2; - @include transition(color, 0.15s, ease-in-out); + @include transition(color 0.15s ease-in-out); + + display: block; margin-top: ($baseline/4); color: $gray-l3; } + .tip-note { + display: block; + margin-top: ($baseline/4); + } + .tip-error { display: none; float: none; @@ -365,7 +354,6 @@ form[class^="create-"] { } } - // form - inline xblock name edit on unit, container, outline // TOOD: abstract this out into a Sass placeholder @@ -402,6 +390,25 @@ form[class^="create-"] { } } +// ELEM: form wrapper +.wrapper-create-element { + height: 0; + margin-bottom: $baseline; + opacity: 0.0; + pointer-events: none; + overflow: hidden; + + &.animate { + @include transition(opacity $tmg-f1 ease-in-out 0s, height $tmg-f1 ease-in-out 0s); + } + + &.is-shown { + height: auto; // define a specific height for the animating version of this UI to work properly + opacity: 1.0; + pointer-events: auto; + } +} + // ==================== // forms - grandfathered diff --git a/cms/static/sass/elements/_system-feedback.scss b/cms/static/sass/elements/_system-feedback.scss index 3326ca4427..034653ad05 100644 --- a/cms/static/sass/elements/_system-feedback.scss +++ b/cms/static/sass/elements/_system-feedback.scss @@ -527,7 +527,7 @@ &.wrapper-alert-warning { box-shadow: 0 1px 1px $white, inset 0 2px 2px $shadow-d1, inset 0 -4px 1px $orange; - [class^="icon"] { + .alert-symbol { color: $orange; } } @@ -535,7 +535,7 @@ &.wrapper-alert-error { box-shadow: 0 1px 1px $white, inset 0 2px 2px $shadow-d1, inset 0 -4px 1px $red-l1; - [class^="icon"] { + .alert-symbol { color: $red-l1; } } @@ -543,7 +543,7 @@ &.wrapper-alert-confirmation { box-shadow: 0 1px 1px $white, inset 0 2px 2px $shadow-d1, inset 0 -4px 1px $green; - [class^="icon"] { + .alert-symbol { color: $green; } } @@ -551,7 +551,7 @@ &.wrapper-alert-announcement { box-shadow: 0 1px 1px $white, inset 0 2px 2px $shadow-d1, inset 0 -4px 1px $blue; - [class^="icon"] { + .alert-symbol { color: $blue; } } @@ -559,7 +559,7 @@ &.wrapper-alert-step-required { box-shadow: 0 1px 1px $white, inset 0 2px 2px $shadow-d1, inset 0 -4px 1px $pink; - [class^="icon"] { + .alert-symbol { color: $pink; } } @@ -579,11 +579,11 @@ @extend %t-strong; } - [class^="icon"], .copy { + .alert-symbol, .copy { float: left; } - [class^="icon"] { + .alert-symbol { @include transition (color 0.50s ease-in-out 0s); @extend %t-icon3; width: flex-grid(1, 12); @@ -605,7 +605,7 @@ // with actions &.has-actions { - [class^="icon"] { + .alert-symbol { width: flex-grid(1, 12); } @@ -667,6 +667,28 @@ background: $gray-d1; } } + + // with dismiss (to sunset action-alert-clos) + .action-dismiss { + + .button { + @extend %btn-secondary-white; + } + + .icon,.button-copy { + display: inline-block; + vertical-align: middle; + } + + .icon { + @extend %t-icon4; + margin-right: ($baseline/4); + } + + .button-copy { + @extend %t-copy-sub1; + } + } } // ==================== diff --git a/cms/static/sass/style-app-extend1.scss b/cms/static/sass/style-app-extend1.scss index bdcf8f79d1..924b7c114f 100644 --- a/cms/static/sass/style-app-extend1.scss +++ b/cms/static/sass/style-app-extend1.scss @@ -38,6 +38,7 @@ @import 'views/dashboard'; @import 'views/export'; @import 'views/index'; +@import 'views/course-create'; @import 'views/import'; @import 'views/outline'; @import 'views/settings'; diff --git a/cms/static/sass/views/_course-create.scss b/cms/static/sass/views/_course-create.scss new file mode 100644 index 0000000000..7b0a500a5f --- /dev/null +++ b/cms/static/sass/views/_course-create.scss @@ -0,0 +1,116 @@ +// studio - views - course creation page +// ==================== + +.view-course-create { + + // basic layout + // -------------------- + .content-primary, .content-supplementary { + @include box-sizing(border-box); + float: left; + } + + .content-primary { + width: flex-grid(9, 12); + margin-right: flex-gutter(); + } + + .content-supplementary { + width: flex-grid(3, 12); + } + + // + + // header/masthead + // -------------------- + .mast .page-header-super { + + .course-original-title-id, .course-original-title { + display: block; + } + + .course-original-title-id { + @extend %t-title5; + } + } + + + // course re-run form + // -------------------- + .rerun-course { + + .row { + @include clearfix(); + margin-bottom: ($baseline*0.75); + } + + .column { + float: left; + width: 48%; + } + + .column:first-child { + margin-right: 4%; + } + + label { + @extend %t-title7; + display: block; + font-weight: 700; + } + + .rerun-course-org, + .rerun-course-number, + .rerun-course-name, + .rerun-course-run { + width: 100%; + } + + .rerun-course-name { + @extend %t-title5; + font-weight: 300; + } + + .rerun-course-save { + @include blue-button; + } + + .rerun-course-cancel { + @include white-button; + } + + .item-details { + padding-bottom: 0; + } + + .wrap-error { + @include transition(opacity $tmg-f2 ease 0s); + opacity: 0; + } + + .wrap-error.is-shown { + opacity: 1; + } + + .message-status { + display: block; + margin-bottom: 0; + padding: ($baseline*.5) ($baseline*1.5) 8px ($baseline*1.5); + font-weight: bold; + } + + // NOTE: override for modern button styling until all buttons (in _forms.scss) can be converted + .actions { + + .action-primary { + @include blue-button; + @extend %t-action2; + } + + .action-secondary { + @include grey-button; + @extend %t-action2; + } + } + } +} diff --git a/cms/static/sass/views/_dashboard.scss b/cms/static/sass/views/_dashboard.scss index 69ba41fd2b..8ca15e4a36 100644 --- a/cms/static/sass/views/_dashboard.scss +++ b/cms/static/sass/views/_dashboard.scss @@ -294,120 +294,298 @@ // ELEM: course listings .courses { margin: $baseline 0; - } + + .title { + @extend %t-title6; + margin-bottom: $baseline; + border-bottom: 1px solid $gray-l3; + padding-bottom: ($baseline/2); + color: $gray-l2; + } + } .list-courses { margin-top: $baseline; border-radius: 3px; - border: 1px solid $gray; + border: 1px solid $gray-l2; background: $white; - box-shadow: 0 1px 2px $shadow-l1; + box-shadow: 0 1px 1px $shadow-l1; - .course-item { - @include box-sizing(border-box); - width: flex-grid(9, 9); - position: relative; - border-bottom: 1px solid $gray-l1; - padding: $baseline; + li:last-child { + margin-bottom: 0; + } + } - // STATE: hover/focus - &:hover { - background: $paleYellow; - .course-actions .view-live-button { - opacity: 1.0; - pointer-events: auto; - } + // UI: course wrappers (needed for status messages) + .wrapper-course { - .course-title { - color: $orange-d1; - } + // CASE: has status + &.has-status { - .course-metadata { - opacity: 1.0; - } - } - - .course-link, .course-actions { + .course-status { @include box-sizing(border-box); display: inline-block; vertical-align: middle; - } - - // encompassing course link - .course-link { - @extend %ui-depth2; - width: flex-grid(7, 9); - margin-right: flex-gutter(); - } - - // course title - .course-title { - @extend %t-title4; - @extend %t-light; - margin: 0 ($baseline*2) ($baseline/4) 0; - } - - // course metadata - .course-metadata { - @extend %t-copy-sub1; - @include transition(opacity $tmg-f1 ease-in-out 0); - color: $gray; - opacity: 0.75; - - .metadata-item { - display: inline-block; - - &:after { - content: "/"; - margin-left: ($baseline/10); - margin-right: ($baseline/10); - color: $gray-l4; - } - - &:last-child { - - &:after { - content: ""; - margin-left: 0; - margin-right: 0; - } - } - - .label { - @extend %cont-text-sr; - } - } - } - - .course-actions { - @extend %ui-depth3; - position: static; - width: flex-grid(2, 9); + width: flex-grid(3, 9); + padding-right: ($baseline/2); text-align: right; - // view live button - .view-live-button { - @extend %ui-depth3; - @include transition(opacity $tmg-f2 ease-in-out 0); - @include box-sizing(border-box); - padding: ($baseline/2); - opacity: 0.0; - pointer-events: none; + .value { - &:hover { - opacity: 1.0; - pointer-events: auto; + .copy, *[class^="icon"] { + display: inline-block; + vertical-align: middle; + } + + *[class^="icon"] { + @extend %t-icon4; + margin-right: ($baseline/2); + } + + .copy { + @extend %t-copy-sub1; } } + } - &:last-child { - border-bottom: none; + .status-message { + @extend %t-copy-sub1; + background-color: $gray-l5; + box-shadow: 0 2px 2px 0 $shadow inset; + padding: ($baseline*0.75) $baseline; + + &.has-actions { + + .copy, .status-actions { + display: inline-block; + vertical-align: middle; + } + + .copy { + width: 65%; + margin: 0 $baseline 0 0; + } + + .status-actions { + width: 30%; + text-align: right; + + .button { + @extend %btn-secondary-white; + } + + .icon,.button-copy { + display: inline-block; + vertical-align: middle; + } + + .icon { + @extend %t-icon4; + margin-right: ($baseline/4); + } + + .button-copy { + @extend %t-copy-sub1; + } + } } } } } + // UI: individual course listings + .course-item { + @include box-sizing(border-box); + width: flex-grid(9, 9); + position: relative; + border-bottom: 1px solid $gray-l2; + padding: $baseline; + + // STATE: hover/focus + &:hover { + background: $paleYellow; + + .course-actions { + opacity: 1.0; + pointer-events: auto; + } + + .course-title { + color: $orange-d1; + } + + .course-metadata { + opacity: 1.0; + } + } + + .course-link, .course-actions { + @include box-sizing(border-box); + display: inline-block; + vertical-align: middle; + } + + // encompassing course link + .course-link { + @extend %ui-depth2; + width: flex-grid(6, 9); + margin-right: flex-gutter(); + } + + // course title + .course-title { + @extend %t-title4; + margin: 0 ($baseline*2) ($baseline/4) 0; + font-weight: 300; + } + + // course metadata + .course-metadata { + @extend %t-copy-sub1; + @include transition(opacity $tmg-f1 ease-in-out 0); + color: $gray; + opacity: 0.75; + + .metadata-item { + display: inline-block; + + &:after { + content: "/"; + margin-left: ($baseline/10); + margin-right: ($baseline/10); + color: $gray-l4; + } + + &:last-child { + + &:after { + content: ""; + margin-left: 0; + margin-right: 0; + } + } + + .label { + @extend %cont-text-sr; + } + } + } + + .course-actions { + @include transition(opacity $tmg-f2 ease-in-out 0); + @extend %ui-depth3; + position: static; + width: flex-grid(3, 9); + text-align: right; + opacity: 0; + pointer-events: none; + + .action { + display: inline-block; + vertical-align: middle; + margin-right: ($baseline/2); + + &:last-child { + margin-right: 0; + } + } + + .button { + @extend %t-action3; + } + + // view live button + .view-button { + @include box-sizing(border-box); + padding: ($baseline/2); + } + + // course re-run button + .action-rerun { + margin-right: $baseline; + } + + .rerun-button { + font-weight: 600; + // TODO: sync up button styling and add secondary style here + } + } + + // CASE: is processing + &.is-processing { + + .course-status .value { + color: $gray-l2; + } + } + + // CASE: has an error + &.has-error { + + .course-status { + color: $red; // TODO: abstract this out to an error-based color variable + } + + ~ .status-message { + background: $red-l1; // TODO: abstract this out to an error-based color variable + color: $white; + } + } + + // CASE: last course in listing + &:last-child { + border-bottom: none; + } + } + + // ==================== + + // CASE: courses that are being processed + .courses-processing { + margin-bottom: ($baseline*2); + border-bottom: 1px solid $gray-l3; + padding-bottom: ($baseline*2); + + // TODO: abstract this case out better with normal course listings + .list-courses { + border: none; + background: none; + box-shadow: none; + } + + .wrapper-course { + @extend %ui-window; + position: relative; + } + + .course-item { + border: none; + + // STATE: hover/focus + &:hover { + background: inherit; + + .course-title { + color: inherit; + } + } + + } + + // course details (replacement for course-link when a course cannot be linked) + .course-details { + @extend %ui-depth2; + display: inline-block; + vertical-align: middle; + width: flex-grid(6, 9); + margin-right: flex-gutter(); + } + + } + + // ==================== + // ELEM: new user form .wrapper-create-course { @@ -494,6 +672,5 @@ margin-bottom: 0; padding: ($baseline*.5) ($baseline*1.5) 8px ($baseline*1.5); } - } } diff --git a/cms/templates/base.html b/cms/templates/base.html index 72b8cfcc0e..a0ca8fc724 100644 --- a/cms/templates/base.html +++ b/cms/templates/base.html @@ -343,7 +343,9 @@ <% online_help_token = self.online_help_token() if hasattr(self, 'online_help_token') else None %> <%include file="widgets/header.html" args="online_help_token=online_help_token" /> -
+
+ <%block name="page_alert"> +
<%block name="content"> diff --git a/cms/templates/course-create-rerun.html b/cms/templates/course-create-rerun.html new file mode 100644 index 0000000000..87f18d3431 --- /dev/null +++ b/cms/templates/course-create-rerun.html @@ -0,0 +1,173 @@ + + +<%inherit file="base.html" /> + +<%! from django.utils.translation import ugettext as _ %> +<%! from django.core.urlresolvers import reverse %> + +<%block name="title">${_("Create a Course Rerun of:")} +<%block name="bodyclass">is-signedin view-course-create view-course-create-rerun + +<%block name="jsextra"> + + + + + +<%block name="content"> +
+
+
+

+ ${_("Create a re-run of a course")} +

+ + + +

+ ${_("You are creating a re-run from:")} + ${source_course_key.org} ${source_course_key.course} ${source_course_key.run} + ${display_name} +

+
+
+ +
+
+
+
+
+
+

+ ${_("Provide identifying information for this re-run of the course. The original course is not affected in any way by a re-run.")} + ${_("Note: Together, the organization, course number, and course run must uniquely identify this new course instance.")} +

+

+
+ + + +
+
+ + + +
+
+ ${_("Required Information to Create a re-run of a course")} + +
    +
  1. + + + + ${_("The public display name for the new course. (This name is often the same as the original course name.)")} + + +
    2. +
  2. + + + + ${_("The name of the organization sponsoring the new course. (This name is often the same as the original organization name.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    3. + +
  3. +
    + + + + ${_("The unique number that identifies the new course within the organization. (This number is often the same as the original course number.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    + +
    + + + + ${_("The term in which the new course will run. (This value is often different than the original course run value.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    +
    4. +
+ + +
+
+ +
+ + +
+
+
+ +
+ + + +
+
+ +
+
+ \ No newline at end of file diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index ee22386600..2fd4867b64 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -36,6 +36,31 @@ from contentstore.utils import reverse_usage_url % endfor +<%block name="page_alert"> + %if notification_dismiss_url is not None: +
+
+ + +
+

This course was created as a re-run. Some manual configuration is needed.

+ +

Be sure to review and reset all dates (the Course Start Date was set to January 1, 2030); set up the course team; review course updates and other assets for dated material; and seed the discussions and wiki.

+
+ + +
+
+ %endif + + <%block name="content">
diff --git a/cms/templates/index.html b/cms/templates/index.html index 0b37c9ecfc..5fa9885fba 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -77,7 +77,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) { % if course_creator_status=='granted':
-
+
% endif + + %if len(unsucceeded_course_actions) > 0: +
+

Courses Being Processed

+ +
    + %for display_name, org, num, run, state_failed, state_in_progress, dismiss_link in sorted(unsucceeded_course_actions, key=lambda s: s[0].lower() if s[0] is not None else ''): + + %if state_in_progress: +
  • +
    +
    +

    ${display_name}

    + +
    + + ${_("Organization:")} ${org} + + + ${_("Course Number:")} + ${num} + + + ${_("Course Run:")} ${run} + +
    +
    + +
    +
    This re-run processing status:
    +
    + + Configuring as re-run +
    +
    +
    + +
    +

    ${_("The new course will be added to your course list in 5-10 minutes. Return to this page or refresh it to update the course list. The new course will need some manual configuration.")}

    +
    +
    • + %endif + + + + + %if state_failed: +
  • +
    +
    +

    ${display_name}

    + +
    + + ${_("Organization:")} ${org} + + + ${_("Course Number:")} + ${num} + + + ${_("Course Run:")} ${run} + +
    +
    + +
    +
    This re-run processing status:
    +
    + + Configuration Error +
    +
    +
    + +
    +

    ${_("A system error occurred while your course was being processed. Please go to the original course to try the re-run again, or contact your PM for assistance.")}

    + + +
    +
    • + %endif + %endfor +
+
+ %endif + %if len(courses) > 0:
diff --git a/cms/templates/manage_users.html b/cms/templates/manage_users.html index 53ba31cb46..c6e410e700 100644 --- a/cms/templates/manage_users.html +++ b/cms/templates/manage_users.html @@ -34,7 +34,7 @@
%if allow_actions:
- +

${_("Add a User to Your Course's Team")}

@@ -147,7 +147,7 @@

${_("Course Team Roles")}

${_("Course team members, or staff, are course co-authors. They have full writing and editing privileges on all course content.")}

-

${_("Admins are course team members who can add and remove other course team members.")}

+

${_("Admins are course team members who can add and remove other course team members.")}

% if user_is_instuctor and len(instructors) == 1: diff --git a/cms/templates/ux/reference/course-create-rerun.html b/cms/templates/ux/reference/course-create-rerun.html new file mode 100644 index 0000000000..89055cd080 --- /dev/null +++ b/cms/templates/ux/reference/course-create-rerun.html @@ -0,0 +1,368 @@ + + +<%inherit file="../../base.html" /> + +<%! from django.utils.translation import ugettext as _ %> +<%! from django.core.urlresolvers import reverse %> + +<%block name="title">[template] ${_("Create a Course Rerun of HarvardX SW12.2x T2_2014")} +<%block name="bodyclass">is-signedin view-course-create view-course-create-rerun + +<%block name="content"> +
+ +
+
+

+ ${_("Create a re-run of a course")} +

+ + + +

+ ${_("You are creating a re-run from:")} + HarvardX SW12.2x T2_2014 + China (Part 2): The Creation and End of a Centralized Empire +

+
+
+ +
+
+
+
+
+
+

+ ${_("Provide identifying information for this re-run of the course. The original course is not affected in any way by a re-run.")} + ${_("Note: Together, the organization, course number, and course run must uniquely identify this new course instance.")} +

+

+
+ + + + +
+ + +
+ +
+ +
+
+ ${_("Required Information to Create a re-run of a course")} + +
    +
  1. + + + + ${_("The public display name for the new course. (This name is often the same as the original course name.)")} + + +
    2. +
  2. + + + + ${_("The name of the organization sponsoring the new course. (This name is often the same as the original organization name.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    3. + +
  3. +
    + + + + ${_("The unique number that identifies the new course within the organization. (This number is often the same as the original course number.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    + +
    + + + + ${_("The term in which the new course will run. (This value is often different than the original course run value.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    +
    4. +
+ + +
+
+ +
+ + +
+ +
+ + + + +
+
+
+ + +
+ +
+
+ ${_("Required Information to Create a re-run of a course")} + +
    +
  1. + + + + ${_("The public display name for the new course. (This name is often the same as the original course name.)")} + + +
    2. +
  2. + + + + ${_("The name of the organization sponsoring the new course. (This name is often the same as the original organization name.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    3. + +
  3. +
    + + + + ${_("The unique number that identifies the new course within the organization. (This number is often the same as the original course number.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    + +
    + + + + ${_("The term in which the new course will run. (This value is often different than the original course run value.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    +
    4. +
+ + + +
+
+ +
+ + +
+
+
+ + + + +
+
+
+ + +
+ +
+
+ ${_("Required Information to Create a re-run of a course")} + +
    +
  1. + + + + ${_("The public display name for the new course. (This name is often the same as the original course name.)")} + + Required field. +
    2. +
  2. + + + + ${_("The name of the organization sponsoring the new course. (This name is often the same as the original organization name.)")} + ${_("Note: No spaces or special characters are allowed.")} + + Please do not use any spaces or special characters in this field. +
    3. + +
  3. +
    + + + + ${_("The unique number that identifies the new course within the organization. (This number is often the same as the original course number.)")} + ${_("Note: No spaces or special characters are allowed.")} + + Please do not use any spaces or special characters in this field. +
    + +
    + + + + ${_("The term in which the new course will run. (This value is often different than the original course run value.)")} + ${_("Note: No spaces or special characters are allowed.")} + + Required field. +
    +
    4. +
+ + + +
+
+ +
+ + +
+
+
+ + + + +
+
+
+
+ +
+
+ ${_("Required Information to Create a re-run of a course")} + +
    +
  1. + + + + ${_("The public display name for the new course. (This name is often the same as the original course name.)")} + + +
    2. +
  2. + + + + ${_("The name of the organization sponsoring the new course. (This name is often the same as the original organization name.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    3. + +
  3. +
    + + + + ${_("The unique number that identifies the new course within the organization. (This number is often the same as the original course number.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    + +
    + + + + ${_("The term in which the new course will run. (This value is often different than the original course run value.)")} + ${_("Note: No spaces or special characters are allowed.")} + + +
    +
    4. +
+ + +
+
+ +
+ +
+
+
+ +
+ + + +
+
+ +
+
+ diff --git a/cms/urls.py b/cms/urls.py index 46e22e3408..ceea5f039f 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -74,6 +74,7 @@ urlpatterns += patterns( ), url(r'^course/{}?$'.format(settings.COURSE_KEY_PATTERN), 'course_handler', name='course_handler'), url(r'^course_notifications/{}/(?P\d+)?$'.format(settings.COURSE_KEY_PATTERN), 'course_notifications_handler'), + url(r'^course_rerun/{}$'.format(settings.COURSE_KEY_PATTERN), 'course_rerun_handler', name='course_rerun_handler'), url(r'^container/{}$'.format(settings.USAGE_KEY_PATTERN), 'container_handler'), url(r'^checklists/{}/(?P\d+)?$'.format(settings.COURSE_KEY_PATTERN), 'checklists_handler'), url(r'^orphan/{}$'.format(settings.COURSE_KEY_PATTERN), 'orphan_handler'), diff --git a/common/djangoapps/course_action_state/managers.py b/common/djangoapps/course_action_state/managers.py index 84ba239213..661f417a69 100644 --- a/common/djangoapps/course_action_state/managers.py +++ b/common/djangoapps/course_action_state/managers.py @@ -113,7 +113,7 @@ class CourseRerunUIStateManager(CourseActionUIStateManager): FAILED = "failed" SUCCEEDED = "succeeded" - def initiated(self, source_course_key, destination_course_key, user): + def initiated(self, source_course_key, destination_course_key, user, display_name): """ To be called when a new rerun is initiated for the given course by the given user. """ @@ -123,6 +123,7 @@ class CourseRerunUIStateManager(CourseActionUIStateManager): user=user, allow_not_found=True, source_course_key=source_course_key, + display_name=display_name, ) def succeeded(self, course_key): diff --git a/common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py b/common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py new file mode 100644 index 0000000000..8710b96bae --- /dev/null +++ b/common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py @@ -0,0 +1,76 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding field 'CourseRerunState.display_name' + db.add_column('course_action_state_coursererunstate', 'display_name', + self.gf('django.db.models.fields.CharField')(default='', max_length=255), + keep_default=False) + + + def backwards(self, orm): + # Deleting field 'CourseRerunState.display_name' + db.delete_column('course_action_state_coursererunstate', 'display_name') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'course_action_state.coursererunstate': { + 'Meta': {'unique_together': "(('course_key', 'action'),)", 'object_name': 'CourseRerunState'}, + 'action': ('django.db.models.fields.CharField', [], {'max_length': '100', 'db_index': 'True'}), + 'course_key': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}), + 'created_time': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'created_user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'created_by_user+'", 'null': 'True', 'on_delete': 'models.SET_NULL', 'to': "orm['auth.User']"}), + 'display_name': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '255'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'message': ('django.db.models.fields.CharField', [], {'max_length': '1000'}), + 'should_display': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'source_course_key': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}), + 'state': ('django.db.models.fields.CharField', [], {'max_length': '50'}), + 'updated_time': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}), + 'updated_user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'updated_by_user+'", 'null': 'True', 'on_delete': 'models.SET_NULL', 'to': "orm['auth.User']"}) + } + } + + complete_apps = ['course_action_state'] \ No newline at end of file diff --git a/common/djangoapps/course_action_state/models.py b/common/djangoapps/course_action_state/models.py index 83e6231ae2..a6a4c23de2 100644 --- a/common/djangoapps/course_action_state/models.py +++ b/common/djangoapps/course_action_state/models.py @@ -109,6 +109,9 @@ class CourseRerunState(CourseActionUIState): # Original course that is being rerun source_course_key = CourseKeyField(max_length=255, db_index=True) + # Display name for destination course + display_name = models.CharField(max_length=255, default="") + # MANAGERS # Override the abstract class' manager with a Rerun-specific manager that inherits from the base class' manager. objects = CourseRerunUIStateManager() diff --git a/common/djangoapps/course_action_state/tests/test_rerun_manager.py b/common/djangoapps/course_action_state/tests/test_rerun_manager.py index 92dfb7dcc0..e94a6a1cb2 100644 --- a/common/djangoapps/course_action_state/tests/test_rerun_manager.py +++ b/common/djangoapps/course_action_state/tests/test_rerun_manager.py @@ -17,10 +17,13 @@ class TestCourseRerunStateManager(TestCase): self.source_course_key = CourseLocator("source_org", "source_course_num", "source_run") self.course_key = CourseLocator("test_org", "test_course_num", "test_run") self.created_user = UserFactory() + self.display_name = "destination course name" self.expected_rerun_state = { 'created_user': self.created_user, 'updated_user': self.created_user, 'course_key': self.course_key, + 'source_course_key': self.source_course_key, + "display_name": self.display_name, 'action': CourseRerunUIStateManager.ACTION, 'should_display': True, 'message': "", @@ -53,10 +56,16 @@ class TestCourseRerunStateManager(TestCase): }) self.verify_rerun_state() - def test_rerun_initiated(self): + def initiate_rerun(self): CourseRerunState.objects.initiated( - source_course_key=self.source_course_key, destination_course_key=self.course_key, user=self.created_user + source_course_key=self.source_course_key, + destination_course_key=self.course_key, + user=self.created_user, + display_name=self.display_name, ) + + def test_rerun_initiated(self): + self.initiate_rerun() self.expected_rerun_state.update( {'state': CourseRerunUIStateManager.State.IN_PROGRESS} ) @@ -64,9 +73,7 @@ class TestCourseRerunStateManager(TestCase): def test_rerun_succeeded(self): # initiate - CourseRerunState.objects.initiated( - source_course_key=self.source_course_key, destination_course_key=self.course_key, user=self.created_user - ) + self.initiate_rerun() # set state to succeed CourseRerunState.objects.succeeded(course_key=self.course_key) @@ -80,9 +87,7 @@ class TestCourseRerunStateManager(TestCase): def test_rerun_failed(self): # initiate - CourseRerunState.objects.initiated( - source_course_key=self.source_course_key, destination_course_key=self.course_key, user=self.created_user - ) + self.initiate_rerun() # set state to fail exception = Exception("failure in rerunning") diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index abe9a693f0..f576ea67e0 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -1794,7 +1794,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): xblock_class = self.mixologist.mix(xblock_class) for field_name, value in fields.iteritems(): - if value: + if value is not None: if isinstance(xblock_class.fields[field_name], Reference): fields[field_name] = value.block_id elif isinstance(xblock_class.fields[field_name], ReferenceList): From 18a6a0487b85e46c62a81b57b5b55e5e561b9d89 Mon Sep 17 00:00:00 2001 From: Mat Peterson Date: Mon, 11 Aug 2014 18:42:36 +0000 Subject: [PATCH 02/42] added JS to outline page after bulk publishing rewrite --- cms/djangoapps/contentstore/views/course.py | 3 ++- cms/static/js/views/pages/course_outline.js | 19 +++++++++++++++++-- cms/templates/index.html | 4 ++-- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 854028c9b7..2bf9efa36a 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -395,7 +395,8 @@ def course_listing(request): 'user': request.user, 'request_course_creator_url': reverse('contentstore.views.request_course_creator'), 'course_creator_status': _get_course_creator_status(request.user), - 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False) + 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False), + 'allow_course_reruns': settings.FEATURES.get('ALLOW_COURSE_RERUNS', True) }) diff --git a/cms/static/js/views/pages/course_outline.js b/cms/static/js/views/pages/course_outline.js index a227642945..62510a6c43 100644 --- a/cms/static/js/views/pages/course_outline.js +++ b/cms/static/js/views/pages/course_outline.js @@ -1,9 +1,9 @@ /** * This page is used to show the user an outline of the course. */ -define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views/utils/xblock_utils", +define(["domReady", "jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views/utils/xblock_utils", "js/views/course_outline"], - function ($, _, gettext, BasePage, XBlockViewUtils, CourseOutlineView) { + function (domReady, $, _, gettext, BasePage, XBlockViewUtils, CourseOutlineView) { var expandedLocators, CourseOutlinePage; CourseOutlinePage = BasePage.extend({ @@ -149,5 +149,20 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views } }; + var dismissNotification = function (e) { + e.preventDefault(); + $.ajax({ + url: $('.dismiss-button').data('dismiss-link'), + type: 'DELETE', + success: function(result) { + $('.wrapper-alert-announcement').removeClass('is-shown').addClass('is-hidden') + } + }); + }; + + domReady(function () { + $('.dismiss-button').bind('click', dismissNotification); + }); + return CourseOutlinePage; }); // end define(); diff --git a/cms/templates/index.html b/cms/templates/index.html index 5fa9885fba..d4f475b63c 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -132,7 +132,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) { % endif - %if len(unsucceeded_course_actions) > 0: + %if allow_course_reruns and len(unsucceeded_course_actions) > 0:

Courses Being Processed

@@ -248,7 +248,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {
    - % if course_creator_status=='granted': + % if allow_course_reruns and course_creator_status=='granted':
  • ${_("Re-run Course")}
    • From 497bb39f889cbe2dd5e061aab6e604cb7ba6549e Mon Sep 17 00:00:00 2001 From: Mat Peterson Date: Tue, 12 Aug 2014 20:32:31 +0000 Subject: [PATCH 03/42] Fixed bug in dismissing errored rerun notification and another with empty reruns fields --- cms/static/js/index.js | 2 +- cms/static/js/views/course_rerun.js | 20 +++++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/cms/static/js/index.js b/cms/static/js/index.js index fe7f79f97a..31c6ef7803 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -4,7 +4,7 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], var dismissNotification = function (e) { e.preventDefault(); $.ajax({ - url: $('.dismiss-button').data('dismiss-link'), + url: this.data('dismiss-link'), type: 'DELETE', success: function(result) { window.location.reload() diff --git a/cms/static/js/views/course_rerun.js b/cms/static/js/views/course_rerun.js index 34e49f37c3..962277e1aa 100644 --- a/cms/static/js/views/course_rerun.js +++ b/cms/static/js/views/course_rerun.js @@ -3,7 +3,7 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], var saveRerunCourse = function (e) { e.preventDefault(); - // One final check for empty values + // One final check for errors var errors = _.reduce( ['.rerun-course-name', '.rerun-course-org', '.rerun-course-number', '.rerun-course-run'], function (acc, ele) { @@ -133,6 +133,18 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], } }; + // Ensure that all fields are not empty + var validateFilledFields = function () { + return _.reduce( + ['.rerun-course-org', '.rerun-course-number', '.rerun-course-run', '.rerun-course-name'], + function (acc, ele) { + var $ele = $(ele); + return $ele.val().length !== 0 ? acc : false; + }, + true + ); + }; + // Handle validation asynchronously _.each( ['.rerun-course-org', '.rerun-course-number', '.rerun-course-run'], @@ -148,6 +160,9 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], var error = validateCourseItemEncoding($ele.val()); setNewCourseFieldInErr($ele.parent(), error); validateTotalCourseItemsLength(); + if(!validateFilledFields()) { + $('.rerun-course-save').addClass('is-disabled'); + } }); } ); @@ -156,6 +171,9 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], var error = validateRequiredField($name.val()); setNewCourseFieldInErr($name.parent(), error); validateTotalCourseItemsLength(); + if(!validateFilledFields()) { + $('.rerun-course-save').addClass('is-disabled'); + } }); }); }); \ No newline at end of file From 64cc16f1eca13d7ec5dff3e40d6e68ce4b5c07dc Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Tue, 12 Aug 2014 20:12:18 -0400 Subject: [PATCH 04/42] Studio: correcting reversion of _base.scss from poor previous rebase --- cms/static/sass/_base.scss | 4 +++- cms/static/sass/elements/_layout.scss | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/cms/static/sass/_base.scss b/cms/static/sass/_base.scss index 547d9b2e39..8f81ccbe10 100644 --- a/cms/static/sass/_base.scss +++ b/cms/static/sass/_base.scss @@ -661,6 +661,7 @@ hr.divide { text-align: center; .close-icon { + @extend %t-strong; @extend %t-action2; @extend %t-strong; } @@ -775,6 +776,7 @@ hr.divide { @include transition(opacity $tmg-f3 ease-out 0s); @include font-size(12); @extend %t-regular; + @extend %t-regular; @extend %ui-depth5; position: absolute; top: 0; @@ -789,7 +791,7 @@ hr.divide { &:after { @include font-size(20); - content: '▾'; + content: '???'; display: block; position: absolute; bottom: -14px; diff --git a/cms/static/sass/elements/_layout.scss b/cms/static/sass/elements/_layout.scss index 1dbd1b0b10..cc3324805c 100644 --- a/cms/static/sass/elements/_layout.scss +++ b/cms/static/sass/elements/_layout.scss @@ -141,6 +141,26 @@ } } + // CASE: wizard-based mast + .mast-wizard { + + .page-header-sub { + @extend %t-title4; + color: $gray; + font-weight: 300; + } + + .page-header-super { + @extend %t-title4; + float: left; + width: flex-grid(12,12); + margin-top: ($baseline/2); + border-top: 1px solid $gray-l4; + padding-top: ($baseline/2); + font-weight: 600; + } + } + // page metadata/action bar .metadata { From 8e3b532768b50c6259221915a12d4561e4813aa9 Mon Sep 17 00:00:00 2001 From: Mat Peterson Date: Wed, 13 Aug 2014 14:26:58 +0000 Subject: [PATCH 05/42] Fixed bug in dismissing rerun failure notification --- cms/static/js/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/static/js/index.js b/cms/static/js/index.js index 31c6ef7803..51fe74ab60 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -4,7 +4,7 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], var dismissNotification = function (e) { e.preventDefault(); $.ajax({ - url: this.data('dismiss-link'), + url: $(this).data('dismiss-link'), type: 'DELETE', success: function(result) { window.location.reload() From d695de24dcf7b913e22127b1cecebc9618fe67c8 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Wed, 13 Aug 2014 10:30:36 -0400 Subject: [PATCH 06/42] Studio: addressing course re-run PR feedback --- cms/static/js/views/course_rerun.js | 4 ++-- cms/static/sass/views/_course-create.scss | 6 ++++++ cms/static/sass/views/_dashboard.scss | 14 +++++++++++--- cms/templates/index.html | 4 ++-- .../ux/reference/course-create-rerun.html | 2 +- 5 files changed, 22 insertions(+), 8 deletions(-) diff --git a/cms/static/js/views/course_rerun.js b/cms/static/js/views/course_rerun.js index 962277e1aa..7361a8303a 100644 --- a/cms/static/js/views/course_rerun.js +++ b/cms/static/js/views/course_rerun.js @@ -52,7 +52,7 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], ); // Go into creating re-run state $('.rerun-course-save').addClass('is-disabled').addClass('is-processing').html( - '' + gettext('Processing Re-run Request') + '' + gettext('Processing Re-run Request') ); $('.action-cancel').addClass('is-hidden'); }; @@ -176,4 +176,4 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], } }); }); - }); \ No newline at end of file + }); diff --git a/cms/static/sass/views/_course-create.scss b/cms/static/sass/views/_course-create.scss index 7b0a500a5f..3b6ce18371 100644 --- a/cms/static/sass/views/_course-create.scss +++ b/cms/static/sass/views/_course-create.scss @@ -73,6 +73,12 @@ .rerun-course-save { @include blue-button; + + .icon { + display: inline-block; + vertical-align: middle; + margin-right: ($baseline/4); + } } .rerun-course-cancel { diff --git a/cms/static/sass/views/_dashboard.scss b/cms/static/sass/views/_dashboard.scss index 8ca15e4a36..3933a999d3 100644 --- a/cms/static/sass/views/_dashboard.scss +++ b/cms/static/sass/views/_dashboard.scss @@ -236,7 +236,7 @@ @extend %t-strong; margin: ($baseline/2); - [class^="icon-"] { + .icon { margin-right: ($baseline/4); } } @@ -295,6 +295,14 @@ .courses { margin: $baseline 0; + .title { + @extend %t-title6; + margin-bottom: $baseline; + border-bottom: 1px solid $gray-l3; + padding-bottom: ($baseline/2); + color: $gray-l2; + } + .title { @extend %t-title6; margin-bottom: $baseline; @@ -333,12 +341,12 @@ .value { - .copy, *[class^="icon"] { + .copy, .icon { display: inline-block; vertical-align: middle; } - *[class^="icon"] { + .icon { @extend %t-icon4; margin-right: ($baseline/2); } diff --git a/cms/templates/index.html b/cms/templates/index.html index d4f475b63c..82497ed0b0 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -162,7 +162,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {
    This re-run processing status:
    - + Configuring as re-run
    @@ -200,7 +200,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {
    This re-run processing status:
    - + Configuration Error
    diff --git a/cms/templates/ux/reference/course-create-rerun.html b/cms/templates/ux/reference/course-create-rerun.html index 89055cd080..b05ee661fe 100644 --- a/cms/templates/ux/reference/course-create-rerun.html +++ b/cms/templates/ux/reference/course-create-rerun.html @@ -328,7 +328,7 @@ TODO:
    From e8763ef71520f36365c60f6507ddd040f56aa405 Mon Sep 17 00:00:00 2001 From: Mat Peterson Date: Wed, 13 Aug 2014 18:16:39 +0000 Subject: [PATCH 07/42] Do TODOs in test_contentstore.py --- .../contentstore/tests/test_contentstore.py | 13 ++++++------- cms/templates/index.html | 6 +++--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 911d1a31bb..caf605fda3 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1584,12 +1584,11 @@ class RerunCourseTest(ContentStoreTestCase): def create_course_listing_html(self, course_key): """Creates html fragment that is created for the given course_key in the course listing section""" - return ' %if state_in_progress: -
  • +

  • ${display_name}

    @@ -178,7 +178,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) { %if state_failed: -
  • +

  • ${display_name}

    @@ -229,7 +229,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {
    diff --git a/cms/templates/index.html b/cms/templates/index.html index bda00eb77e..b89d8c75fe 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -123,7 +123,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {
    - +
    @@ -132,7 +132,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) { % endif - %if allow_course_reruns and len(unsucceeded_course_actions) > 0: + %if allow_course_reruns and rerun_creator_status and len(unsucceeded_course_actions) > 0:

    Courses Being Processed

    @@ -248,7 +248,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {
      - % if allow_course_reruns and course_creator_status=='granted': + % if allow_course_reruns and rerun_creator_status and course_creator_status=='granted':
    • ${_("Re-run Course")}
      • @@ -259,7 +259,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {
    • %endfor - % if course_creator_status=='granted': + % if allow_course_reruns and rerun_creator_status and course_creator_status=='granted': From 6096bae755ddf37b9e0b2656c188ac415c0d8f36 Mon Sep 17 00:00:00 2001 From: Mat Peterson Date: Thu, 14 Aug 2014 15:32:56 +0000 Subject: [PATCH 09/42] Skipping rerun test on feature flag conditional --- cms/djangoapps/contentstore/tests/test_contentstore.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index caf605fda3..c5b72868ea 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -47,6 +47,9 @@ from student.roles import CourseCreatorRole, CourseInstructorRole from opaque_keys import InvalidKeyError from contentstore.tests.utils import get_url from course_action_state.models import CourseRerunState, CourseRerunUIStateManager + +from unittest import skipIf + from course_action_state.managers import CourseActionStateItemNotFoundError @@ -1632,6 +1635,7 @@ class RerunCourseTest(ContentStoreTestCase): self.assertInCourseListing(source_course.id) self.assertInCourseListing(destination_course_key) + @skipIf(not settings.FEATURES.get('ALLOW_COURSE_RERUNS', False), "ALLOW_COURSE_RERUNS are not enabled") def test_rerun_course_fail_no_source_course(self): existent_course_key = CourseFactory.create().id non_existent_course_key = CourseLocator("org", "non_existent_course", "non_existent_run") From 4fcb6c925ab26feddd22604e7284afae39cc60e0 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Thu, 14 Aug 2014 16:15:48 -0400 Subject: [PATCH 10/42] Add jasmine tests for course_rerun.js --- cms/static/coffee/spec/main.coffee | 1 + .../js/spec/views/pages/course_rerun_spec.js | 133 ++++++++++++++++++ cms/static/js/views/course_rerun.js | 17 ++- .../mock/mock-create-course-rerun.underscore | 116 +++++++++++++++ 4 files changed, 264 insertions(+), 3 deletions(-) create mode 100644 cms/static/js/spec/views/pages/course_rerun_spec.js create mode 100644 cms/templates/js/mock/mock-create-course-rerun.underscore diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index 2854eabe34..d80ab05764 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -233,6 +233,7 @@ define([ "js/spec/views/pages/container_subviews_spec", "js/spec/views/pages/group_configurations_spec", "js/spec/views/pages/course_outline_spec", + "js/spec/views/pages/course_rerun_spec", "js/spec/views/modals/base_modal_spec", "js/spec/views/modals/edit_xblock_spec", diff --git a/cms/static/js/spec/views/pages/course_rerun_spec.js b/cms/static/js/spec/views/pages/course_rerun_spec.js new file mode 100644 index 0000000000..077c395313 --- /dev/null +++ b/cms/static/js/spec/views/pages/course_rerun_spec.js @@ -0,0 +1,133 @@ +define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers", "js/views/course_rerun"], + function ($, create_sinon, view_helpers, CourseRerunPage) { + describe("Create course rerun page", function () { + var selectors = { + courseOrg: '.rerun-course-org', + courseNumber: '.rerun-course-number', + courseRun: '.rerun-course-run', + courseName: '.rerun-course-name', + errorField: '.tip-error', + saveButton: '.rerun-course-save', + cancelButton: '.rerun-course-cancel', + errorMessage: '.wrapper-error' + }, + classes = { + hidden: 'is-hidden', + error: 'error', + disabled: 'is-disabled', + processing: 'is-processing' + }, + mockCreateCourseRerunHTML = readFixtures('mock/mock-create-course-rerun.underscore'); + + var fillInFields = function (org, number, run, name) { + $(selectors.courseOrg).val(org); + $(selectors.courseNumber).val(number); + $(selectors.courseRun).val(run); + $(selectors.courseName).val(name); + }; + + beforeEach(function () { + view_helpers.installMockAnalytics(); + window.source_course_key = 'test_course_key'; + appendSetFixtures(mockCreateCourseRerunHTML); + CourseRerunPage.onReady(); + }); + + afterEach(function () { + view_helpers.removeMockAnalytics(); + delete window.source_course_key; + }); + + describe("validateRequiredField", function () { + it("has a message for an empty string", function () { + var message = CourseRerunPage.validateRequiredField(''); + expect(message).not.toBe(''); + }); + + it("does not have a message for a non empty string", function () { + var message = CourseRerunPage.validateRequiredField('edX'); + expect(message).toBe(''); + }); + }); + + describe("setNewCourseFieldInErr", function () { + var setErrorMessage = function(selector, message) { + var element = $(selector).parent(); + CourseRerunPage.setNewCourseFieldInErr(element, message); + return element; + }; + + it("can show an error message", function () { + var element = setErrorMessage(selectors.courseOrg, 'error message'); + expect(element).toHaveClass(classes.error); + expect(element.children(selectors.errorField)).not.toHaveClass(classes.hidden); + expect(element.children(selectors.errorField)).toContainText('error message'); + }); + + it("can hide an error message", function () { + var element = setErrorMessage(selectors.courseOrg, ''); + expect(element).not.toHaveClass(classes.error); + expect(element.children(selectors.errorField)).toHaveClass(classes.hidden); + }); + + it("disables the save button", function () { + setErrorMessage(selectors.courseOrg, 'error message'); + expect($(selectors.saveButton)).toHaveClass(classes.disabled); + }); + + it("enables the save button when all errors are removed", function () { + setErrorMessage(selectors.courseOrg, 'error message 1'); + setErrorMessage(selectors.courseNumber, 'error message 2'); + expect($(selectors.saveButton)).toHaveClass(classes.disabled); + setErrorMessage(selectors.courseOrg, ''); + setErrorMessage(selectors.courseNumber, ''); + expect($(selectors.saveButton)).not.toHaveClass(classes.disabled); + }); + + it("does not enable the save button when errors remain", function () { + setErrorMessage(selectors.courseOrg, 'error message 1'); + setErrorMessage(selectors.courseNumber, 'error message 2'); + expect($(selectors.saveButton)).toHaveClass(classes.disabled); + setErrorMessage(selectors.courseOrg, ''); + expect($(selectors.saveButton)).toHaveClass(classes.disabled); + }); + }); + + it("can save course reruns", function () { + var requests = create_sinon.requests(this); + window.source_course_key = 'test_course_key'; + fillInFields('DemoX', 'DM101', '2014', 'Demo course'); + $(selectors.saveButton).click(); + create_sinon.expectJsonRequest(requests, 'POST', '/course/', { + source_course_key: 'test_course_key', + org: 'DemoX', + number: 'DM101', + run: '2014', + display_name: 'Demo course' + }); + expect($(selectors.saveButton)).toHaveClass(classes.disabled); + expect($(selectors.saveButton)).toHaveClass(classes.processing); + expect($(selectors.cancelButton)).toHaveClass(classes.hidden); + }); + + it("displays an error when saving fails", function () { + var requests = create_sinon.requests(this); + fillInFields('DemoX', 'DM101', '2014', 'Demo course'); + $(selectors.saveButton).click(); + create_sinon.respondWithJson(requests, { + ErrMsg: 'error message' + }); + expect($(selectors.errorMessage)).not.toHaveClass(classes.hidden); + expect($(selectors.errorMessage)).toContainText('error message'); + expect($(selectors.saveButton)).not.toHaveClass(classes.processing); + expect($(selectors.cancelButton)).not.toHaveClass(classes.hidden); + }); + + it("does not save if there are validation errors", function () { + var requests = create_sinon.requests(this); + fillInFields('DemoX', 'DM101', '', 'Demo course'); + $(selectors.saveButton).click(); + expect(requests.length).toBe(0); + }); + }); + }); diff --git a/cms/static/js/views/course_rerun.js b/cms/static/js/views/course_rerun.js index 7361a8303a..57bba6351c 100644 --- a/cms/static/js/views/course_rerun.js +++ b/cms/static/js/views/course_rerun.js @@ -1,4 +1,4 @@ -require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], +define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], function (domReady, $, _, CancelOnEscape) { var saveRerunCourse = function (e) { @@ -87,7 +87,7 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], } }; - domReady(function () { + var onReady = function () { var $cancelButton = $('.rerun-course-cancel'); var $courseRun = $('.rerun-course-run'); $courseRun.focus().select(); @@ -175,5 +175,16 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], $('.rerun-course-save').addClass('is-disabled'); } }); - }); + }; + + domReady(onReady); + + // Return these functions so that they can be tested + return { + saveRerunCourse: saveRerunCourse, + cancelRerunCourse: cancelRerunCourse, + validateRequiredField: validateRequiredField, + setNewCourseFieldInErr: setNewCourseFieldInErr, + onReady: onReady + }; }); diff --git a/cms/templates/js/mock/mock-create-course-rerun.underscore b/cms/templates/js/mock/mock-create-course-rerun.underscore new file mode 100644 index 0000000000..43b9ff0b26 --- /dev/null +++ b/cms/templates/js/mock/mock-create-course-rerun.underscore @@ -0,0 +1,116 @@ +
    +
    +
    +

    + Create a re-run of a course +

    + + + +

    + You are creating a re-run from: + edX Open_DemoX 2014_T1 + edX Demonstration Course +

    +
    +
    + +
    +
    +
    +
    +
    +
    +

    + Provide identifying information for this re-run of the course. The original course is not affected in any way by a re-run. + Note: Together, the organization, course number, and course run must uniquely identify this new course instance. +

    +

    +
    + +
    +
    + + +
    +
    + Required Information to Create a re-run of a course + +
      +
    1. + + + + The public display name for the new course. (This name is often the same as the original course name.) + + +
      2. +
    2. + + + + The name of the organization sponsoring the new course. (This name is often the same as the original organization name.) + Note: No spaces or special characters are allowed. + + +
      3. + +
    3. +
      + + + + The unique number that identifies the new course within the organization. (This number is often the same as the original course number.) + Note: No spaces or special characters are allowed. + + +
      + +
      + + + + The term in which the new course will run. (This value is often different than the original course run value.) + Note: No spaces or special characters are allowed. + + +
      +
      4. +
    + + +
    +
    + +
    + + +
    +
    +
    +
    +
    +
    +
    +
    \ No newline at end of file From 319b53bb3c2bb597fb4f94964e5b76615dfda874 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Thu, 14 Aug 2014 17:15:27 -0400 Subject: [PATCH 11/42] Add jasmine test for notification dismissal on course outline --- .../spec/views/pages/course_outline_spec.js | 15 +++++++- cms/static/js/views/pages/course_outline.js | 34 +++++++++---------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/cms/static/js/spec/views/pages/course_outline_spec.js b/cms/static/js/spec/views/pages/course_outline_spec.js index 352c201d9a..a13537051d 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -7,7 +7,8 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" getItemsOfType, getItemHeaders, verifyItemsExpanded, expandItemsAndVerifyState, collapseItemsAndVerifyState, createMockCourseJSON, createMockSectionJSON, createMockSubsectionJSON, verifyTypePublishable, mockCourseJSON, mockEmptyCourseJSON, mockSingleSectionCourseJSON, createMockVerticalJSON, - mockOutlinePage = readFixtures('mock/mock-course-outline-page.underscore'); + mockOutlinePage = readFixtures('mock/mock-course-outline-page.underscore'), + mockRerunNotification = readFixtures('mock/mock-course-rerun-notification.underscore'); createMockCourseJSON = function(options, children) { return $.extend(true, {}, { @@ -243,6 +244,18 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" }); }); + describe("Rerun notification", function () { + it("can be dismissed", function () { + appendSetFixtures(mockRerunNotification); + createCourseOutlinePage(this, mockEmptyCourseJSON); + expect($('.wrapper-alert-announcement')).not.toHaveClass('is-hidden'); + $('.dismiss-button').click(); + create_sinon.expectJsonRequest(requests, 'DELETE', 'dummy_dismiss_url'); + create_sinon.respondToDelete(requests); + expect($('.wrapper-alert-announcement')).toHaveClass('is-hidden'); + }); + }); + describe("Button bar", function() { it('can add a section', function() { createCourseOutlinePage(this, mockEmptyCourseJSON); diff --git a/cms/static/js/views/pages/course_outline.js b/cms/static/js/views/pages/course_outline.js index 62510a6c43..464c9d5516 100644 --- a/cms/static/js/views/pages/course_outline.js +++ b/cms/static/js/views/pages/course_outline.js @@ -1,9 +1,9 @@ /** * This page is used to show the user an outline of the course. */ -define(["domReady", "jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views/utils/xblock_utils", +define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views/utils/xblock_utils", "js/views/course_outline"], - function (domReady, $, _, gettext, BasePage, XBlockViewUtils, CourseOutlineView) { + function ($, _, gettext, BasePage, XBlockViewUtils, CourseOutlineView) { var expandedLocators, CourseOutlinePage; CourseOutlinePage = BasePage.extend({ @@ -25,6 +25,7 @@ define(["domReady", "jquery", "underscore", "gettext", "js/views/pages/base_page self.outlineView.handleAddEvent(event); }); this.model.on('change', this.setCollapseExpandVisibility, this); + $('.dismiss-button').bind('click', this.dismissNotification) }, setCollapseExpandVisibility: function() { @@ -97,6 +98,20 @@ define(["domReady", "jquery", "underscore", "gettext", "js/views/pages/base_page } }, this); } + }, + + /** + * Dismiss the course rerun notification. + */ + dismissNotification: function (e) { + e.preventDefault(); + $.ajax({ + url: $('.dismiss-button').data('dismiss-link'), + type: 'DELETE', + success: function(result) { + $('.wrapper-alert-announcement').removeClass('is-shown').addClass('is-hidden') + } + }); } }); @@ -149,20 +164,5 @@ define(["domReady", "jquery", "underscore", "gettext", "js/views/pages/base_page } }; - var dismissNotification = function (e) { - e.preventDefault(); - $.ajax({ - url: $('.dismiss-button').data('dismiss-link'), - type: 'DELETE', - success: function(result) { - $('.wrapper-alert-announcement').removeClass('is-shown').addClass('is-hidden') - } - }); - }; - - domReady(function () { - $('.dismiss-button').bind('click', dismissNotification); - }); - return CourseOutlinePage; }); // end define(); From 6c8b418331a85febc77c33846254056e03fbbf7c Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Fri, 15 Aug 2014 11:15:09 -0400 Subject: [PATCH 12/42] Test notification dismissal on course listing page --- cms/static/coffee/spec/main.coffee | 1 + cms/static/js/index.js | 14 +- cms/static/js/spec/views/pages/index_spec.js | 24 +++ cms/static/js/views/course_rerun.js | 5 +- .../mock-course-rerun-notification.underscore | 24 +++ .../js/mock/mock-index-page.underscore | 168 ++++++++++++++++++ 6 files changed, 229 insertions(+), 7 deletions(-) create mode 100644 cms/static/js/spec/views/pages/index_spec.js create mode 100644 cms/templates/js/mock/mock-course-rerun-notification.underscore create mode 100644 cms/templates/js/mock/mock-index-page.underscore diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index d80ab05764..9bd64c5a2a 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -234,6 +234,7 @@ define([ "js/spec/views/pages/group_configurations_spec", "js/spec/views/pages/course_outline_spec", "js/spec/views/pages/course_rerun_spec", + "js/spec/views/pages/index_spec", "js/spec/views/modals/base_modal_spec", "js/spec/views/modals/edit_xblock_spec", diff --git a/cms/static/js/index.js b/cms/static/js/index.js index bf7feb89b3..de124f224b 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -1,4 +1,4 @@ -require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], +define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], function (domReady, $, _, CancelOnEscape) { var dismissNotification = function (e) { @@ -172,9 +172,15 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], } }; - - domReady(function () { + var onReady = function () { $('.new-course-button').bind('click', addNewCourse); $('.dismiss-button').bind('click', dismissNotification); - }); + }; + + domReady(onReady); + + return { + dismissNotification: dismissNotification, + onReady: onReady + }; }); diff --git a/cms/static/js/spec/views/pages/index_spec.js b/cms/static/js/spec/views/pages/index_spec.js new file mode 100644 index 0000000000..bffe8b5e3b --- /dev/null +++ b/cms/static/js/spec/views/pages/index_spec.js @@ -0,0 +1,24 @@ +define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers", "js/index"], + function ($, create_sinon, view_helpers, IndexPage) { + describe("Course listing page", function () { + var mockIndexPageHTML = readFixtures('mock/mock-index-page.underscore'); + + beforeEach(function () { + view_helpers.installMockAnalytics(); + appendSetFixtures(mockIndexPageHTML); + IndexPage.onReady(); + }); + + afterEach(function () { + view_helpers.removeMockAnalytics(); + delete window.source_course_key; + }); + + + it("can dismiss notifications", function () { + var requests = create_sinon.requests(this); + $('.dismiss-button').click(); + create_sinon.expectJsonRequest(requests, 'DELETE', 'dummy_dismiss_url'); + }); + }); + }); \ No newline at end of file diff --git a/cms/static/js/views/course_rerun.js b/cms/static/js/views/course_rerun.js index 57bba6351c..529e9029a9 100644 --- a/cms/static/js/views/course_rerun.js +++ b/cms/static/js/views/course_rerun.js @@ -1,5 +1,5 @@ -define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], - function (domReady, $, _, CancelOnEscape) { +define(["domReady", "jquery", "underscore"], + function (domReady, $, _) { var saveRerunCourse = function (e) { e.preventDefault(); @@ -93,7 +93,6 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], $courseRun.focus().select(); $('.rerun-course-save').on('click', saveRerunCourse); $cancelButton.bind('click', cancelRerunCourse); - CancelOnEscape($cancelButton); $('.cancel-button').bind('click', cancelRerunCourse); // Check that a course (org, number, run) doesn't use any special characters diff --git a/cms/templates/js/mock/mock-course-rerun-notification.underscore b/cms/templates/js/mock/mock-course-rerun-notification.underscore new file mode 100644 index 0000000000..68a0ddea8b --- /dev/null +++ b/cms/templates/js/mock/mock-course-rerun-notification.underscore @@ -0,0 +1,24 @@ +
    +
    +
    + + +
    +

    This course was created as a re-run. Some manual configuration is needed.

    + +

    Be sure to review and reset all dates (the Course Start Date was set to January 1, 2030); set up the + course team; review course updates and other assets for dated material; and seed the discussions and + wiki.

    +
    + + +
    +
    +
    \ No newline at end of file diff --git a/cms/templates/js/mock/mock-index-page.underscore b/cms/templates/js/mock/mock-index-page.underscore new file mode 100644 index 0000000000..f63f12e571 --- /dev/null +++ b/cms/templates/js/mock/mock-index-page.underscore @@ -0,0 +1,168 @@ +
    +
    +

    My Courses

    + +
    +
    + +
    +
    +
    + +
    +

    Welcome, user!

    +
    +

    Here are all of the courses you currently have access to in Studio:

    +
    +
    + +
    +
    +
    + +
    + +
    +

    Create a New Course

    + +
    + Required Information to Create a New Course + +
      +
    1. + + + The public display name for your course. This cannot be changed, but you can set a different display name in Advanced Settings later. + +
      2. +
    2. + + + The name of the organization sponsoring the course. Note: This is part of your course URL, so no spaces or special characters are allowed. This cannot be changed, but you can set a different display name in Advanced Settings later. + +
      3. + +
    3. + + + The unique number that identifies your course within your organization. Note: This is part of your course URL, so no spaces or special characters are allowed and it cannot be changed. + +
      4. + +
    4. + + + The term in which your course will run. Note: This is part of your course URL, so no spaces or special characters are allowed and it cannot be changed. + +
      5. +
    + +
    +
    + +
    + + + +
    +
    +
    + + +
    +

    Courses Being Processed

    + +
      + +
    • +
      +
      +

      Demo Course

      + +
      + + Organization: edX + + + Course Number: + DM101 + + + Course Run: 2014 + +
      +
      + +
      +
      This re-run processing status:
      +
      + + Configuring as re-run +
      +
      +
      + +
      +

      The new course will be added to your course list in 5-10 minutes. Return to this page or refresh it to update the course list. The new course will need some manual configuration.

      +
      +
      • + + + + +
    • +
      +
      +

      Demo Course 2

      + +
      + + Organization: edX + + + Course Number: + DM102 + + + Course Run: 2014 + +
      +
      + +
      +
      This re-run processing status:
      +
      + + Configuration Error +
      +
      +
      + +
      +

      A system error occurred while your course was being processed. Please go to the original course to try the re-run again, or contact your PM for assistance.

      + + +
      +
      • +
    +
    +
    +
    +
    From e3fa5f08799a99ec308eb790c759cadc78334fb7 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Fri, 15 Aug 2014 10:23:14 -0400 Subject: [PATCH 13/42] Studio: removing static rendering notes/comments from course rerun template --- cms/templates/course-create-rerun.html | 31 +++++--------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/cms/templates/course-create-rerun.html b/cms/templates/course-create-rerun.html index 2994fcf4e9..fabb36d251 100644 --- a/cms/templates/course-create-rerun.html +++ b/cms/templates/course-create-rerun.html @@ -1,20 +1,3 @@ - - <%inherit file="base.html" /> <%! from django.utils.translation import ugettext as _ %> @@ -58,7 +41,7 @@ require(["domReady!", "jquery", "jquery.form", "js/views/course_rerun"], functio ${display_name}
-
+
@@ -71,9 +54,7 @@ require(["domReady!", "jquery", "jquery.form", "js/views/course_rerun"], functio ${_("Note: Together, the organization, course number, and course run must uniquely identify this new course instance.")}

-
- - +
@@ -140,7 +121,7 @@ require(["domReady!", "jquery", "jquery.form", "js/views/course_rerun"], functio
- + + - + - \ No newline at end of file + From 7baf338cdb6d56748dd90957b78c549aa8af50ce Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Fri, 15 Aug 2014 11:21:23 -0400 Subject: [PATCH 14/42] Studio: revising help copy and actions on course re-reun UI --- cms/static/sass/elements/_system-help.scss | 6 ++++++ cms/templates/course-create-rerun.html | 10 +++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/cms/static/sass/elements/_system-help.scss b/cms/static/sass/elements/_system-help.scss index f7e34f2043..0538bf6f7d 100644 --- a/cms/static/sass/elements/_system-help.scss +++ b/cms/static/sass/elements/_system-help.scss @@ -243,6 +243,12 @@ } } + // learn more (aka external help button) + .external-help-button { + @extend %ui-btn-flat-outline; + @extend %sizing; + } + // actions .list-actions { @extend %cont-no-list; diff --git a/cms/templates/course-create-rerun.html b/cms/templates/course-create-rerun.html index fabb36d251..eef8e797da 100644 --- a/cms/templates/course-create-rerun.html +++ b/cms/templates/course-create-rerun.html @@ -127,23 +127,27 @@ require(["domReady!", "jquery", "jquery.form", "js/views/course_rerun"], functio

${_("When will my course re-run start?")}

    -
  • ${_("Vivamus sagittis lacus vel augue laoreet rutrum faucibus dolor auctor. Sed posuere consectetur est at lobortis. Maecenas faucibus mollis interdum.")}
    • +
  • ${_("The new course is set to start on January 1, 2030 at midnight (UTC).")}

${_("What transfers from the original course?")}

    -
  • ${_("Vivamus sagittis lacus vel augue laoreet rutrum faucibus dolor auctor. Sed posuere consectetur est at lobortis. Maecenas faucibus mollis interdum.")}
    • +
  • ${_("The new course has the same course outline and content as the original course. All problems, videos, announcements, and other files are duplicated to the new course.")}

${_("What does not transfer from the original course?")}

    -
  • ${_("Vivamus sagittis lacus vel augue laoreet rutrum faucibus dolor auctor. Sed posuere consectetur est at lobortis. Maecenas faucibus mollis interdum.")}
    • +
  • ${_("You are the only member of the new course's staff. No students are enrolled in the course, and there is no student data. There is no content in the discussion topics or wiki.")}
+ +
+ ${_("Learn more about Course Re-runs")} +
From b43c7571016ff3c83fbd89a69a0d67aa36e4d08e Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Fri, 15 Aug 2014 11:21:47 -0400 Subject: [PATCH 15/42] Studio: revising visual padding on dismiss UI used in course re-run flow --- cms/static/sass/elements/_system-feedback.scss | 1 + cms/static/sass/views/_dashboard.scss | 1 + 2 files changed, 2 insertions(+) diff --git a/cms/static/sass/elements/_system-feedback.scss b/cms/static/sass/elements/_system-feedback.scss index 034653ad05..37c1df0aa6 100644 --- a/cms/static/sass/elements/_system-feedback.scss +++ b/cms/static/sass/elements/_system-feedback.scss @@ -673,6 +673,7 @@ .button { @extend %btn-secondary-white; + padding:($baseline/4) ($baseline/2); } .icon,.button-copy { diff --git a/cms/static/sass/views/_dashboard.scss b/cms/static/sass/views/_dashboard.scss index 3933a999d3..167cdde7bb 100644 --- a/cms/static/sass/views/_dashboard.scss +++ b/cms/static/sass/views/_dashboard.scss @@ -381,6 +381,7 @@ .button { @extend %btn-secondary-white; + padding:($baseline/4) ($baseline/2); } .icon,.button-copy { From 972b392d620b91e0db09728da4b798546779d155 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Fri, 15 Aug 2014 12:41:39 -0400 Subject: [PATCH 16/42] Studio: adding in doc URL logic for course re-run help link --- cms/templates/course-create-rerun.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cms/templates/course-create-rerun.html b/cms/templates/course-create-rerun.html index eef8e797da..04bd8044b7 100644 --- a/cms/templates/course-create-rerun.html +++ b/cms/templates/course-create-rerun.html @@ -1,5 +1,5 @@ <%inherit file="base.html" /> - +<%def name="online_help_token()"><% return "course_rerun" %> <%! from django.utils.translation import ugettext as _ %> <%! from django.core.urlresolvers import reverse %> @@ -146,8 +146,8 @@ require(["domReady!", "jquery", "jquery.form", "js/views/course_rerun"], functio
- ${_("Learn more about Course Re-runs")} -
+ ${_("Learn more about Course Re-runs")} +
From aa799a1736132abb5de62f515bcd8a1bb9d7ee9b Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Fri, 15 Aug 2014 12:43:08 -0400 Subject: [PATCH 17/42] Studio: syncing up markup/styling for other 'learn more' external help links --- cms/templates/textbooks.html | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cms/templates/textbooks.html b/cms/templates/textbooks.html index 2a65244c72..7334d28754 100644 --- a/cms/templates/textbooks.html +++ b/cms/templates/textbooks.html @@ -75,7 +75,10 @@ require(["js/models/section", "js/collections/textbook", "js/views/list_textbook

${_("What if my book isn't divided into chapters?")}

${_("If your textbook doesn't have individual chapters, you can upload the entire text as a single chapter and enter a name of your choice in the Chapter Name field.")}

-

${_("Learn More")}

+
+ +
+ ${_("Learn More")}
From 620218b19df1c23ef2fff4b0f56961ae15fa9afc Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Fri, 15 Aug 2014 16:27:15 -0400 Subject: [PATCH 18/42] Studio: removing redundant font-weight Sass placeholders --- cms/static/sass/_base.scss | 2 -- 1 file changed, 2 deletions(-) diff --git a/cms/static/sass/_base.scss b/cms/static/sass/_base.scss index 8f81ccbe10..7885243985 100644 --- a/cms/static/sass/_base.scss +++ b/cms/static/sass/_base.scss @@ -661,7 +661,6 @@ hr.divide { text-align: center; .close-icon { - @extend %t-strong; @extend %t-action2; @extend %t-strong; } @@ -776,7 +775,6 @@ hr.divide { @include transition(opacity $tmg-f3 ease-out 0s); @include font-size(12); @extend %t-regular; - @extend %t-regular; @extend %ui-depth5; position: absolute; top: 0; From 71c41cfb9239476816d64e9582e85e67edca0705 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Mon, 18 Aug 2014 16:14:54 -0400 Subject: [PATCH 19/42] fixup! Set GlobalStaff permissions on reruns --- cms/envs/dev.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cms/envs/dev.py b/cms/envs/dev.py index d1208c0080..fd0249d313 100644 --- a/cms/envs/dev.py +++ b/cms/envs/dev.py @@ -159,6 +159,7 @@ DEBUG_TOOLBAR_MONGO_STACKTRACES = False # Enable URL that shows information about the status of variuous services FEATURES['ENABLE_SERVICE_STATUS'] = True +FEATURES['ALLOW_COURSE_RERUNS'] = True ############################# SEGMENT-IO ################################## From 0d1669c612f1098cc2790d2b748075f123b77e49 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Mon, 18 Aug 2014 16:16:02 -0400 Subject: [PATCH 20/42] DateTime fields can support datetime values. LMS-11209 --- requirements/edx/github.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 7f0e3ef66d..82bdd87389 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -18,7 +18,7 @@ -e git+https://github.com/jazkarta/edx-jsme.git@813079fd5218ed275248d2a1fcae2fcbf20a0838#egg=edx-jsme # Our libraries: --e git+https://github.com/edx/XBlock.git@f0e53538be7ce90584a03cc7dd3f06bd43e12ac2#egg=XBlock +-e git+https://github.com/edx/XBlock.git@de7fde7f27b1f4a0bb7b6ea9041cc893021be287#egg=XBlock -e git+https://github.com/edx/codejail.git@71f5c5616e2a73ae8cecd1ff2362774a773d3665#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.5.0#egg=diff_cover -e git+https://github.com/edx/js-test-tool.git@v0.1.5#egg=js_test_tool From 46932ee0f91fc43c7f1bb7266b6aa9af5473977b Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Mon, 18 Aug 2014 17:20:41 -0400 Subject: [PATCH 21/42] Replace data-test-course and data-test-unsucceeded with data-course-key --- .../contentstore/tests/test_contentstore.py | 25 +++++----- cms/djangoapps/contentstore/views/course.py | 47 ++++++++++--------- cms/templates/index.html | 46 +++++++++--------- requirements/edx/base.txt | 1 + 4 files changed, 62 insertions(+), 57 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index c5b72868ea..946500e299 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -5,6 +5,7 @@ import copy import mock import shutil +import lxml from datetime import timedelta from fs.osfs import OSFS @@ -1585,31 +1586,31 @@ class RerunCourseTest(ContentStoreTestCase): destination_course_key = CourseKey.from_string(json_resp['destination_course_key']) return destination_course_key - def create_course_listing_html(self, course_key): - """Creates html fragment that is created for the given course_key in the course listing section""" - return 'data-test-course="{}/{}/{}"'.format(course_key.org, course_key.course, course_key.run) + def get_course_listing_elements(self, html, course_key): + """Returns the elements in the course listing section of html that have the given course_key""" + return html.cssselect('.course-item[data-course-key="{}"]'.format(unicode(course_key))) - def create_unsucceeded_course_action_html(self, course_key): - """Creates html fragment that is created for the given course_key in the unsucceeded course action section""" - return 'data-test-unsucceeded="{}/{}/{}"'.format(course_key.org, course_key.course, course_key.run) + def get_unsucceeded_course_action_elements(self, html, course_key): + """Returns the elements in the unsucceeded course action section that have the given course_key""" + return html.cssselect('.courses-processing li[data-course-key="{}"]'.format(unicode(course_key))) def assertInCourseListing(self, course_key): """ Asserts that the given course key is in the accessible course listing section of the html and NOT in the unsucceeded course action section of the html. """ - course_listing_html = self.client.get_html('/course/') - self.assertIn(self.create_course_listing_html(course_key), course_listing_html.content) - self.assertNotIn(self.create_unsucceeded_course_action_html(course_key), course_listing_html.content) + course_listing = lxml.html.fromstring(self.client.get_html('/course/').content) + self.assertEqual(len(self.get_course_listing_elements(course_listing, course_key)), 1) + self.assertEqual(len(self.get_unsucceeded_course_action_elements(course_listing, course_key)), 0) def assertInUnsucceededCourseActions(self, course_key): """ Asserts that the given course key is in the unsucceeded course action section of the html and NOT in the accessible course listing section of the html. """ - course_listing_html = self.client.get_html('/course/') - self.assertNotIn(self.create_course_listing_html(course_key), course_listing_html.content) - self.assertIn(self.create_unsucceeded_course_action_html(course_key), course_listing_html.content) + course_listing = lxml.html.fromstring(self.client.get_html('/course/').content) + self.assertEqual(len(self.get_course_listing_elements(course_listing, course_key)), 0) + self.assertEqual(len(self.get_unsucceeded_course_action_elements(course_listing, course_key)), 1) def test_rerun_course_success(self): diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 0443f92827..33f083a020 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -353,33 +353,36 @@ def course_listing(request): def format_course_for_view(course): """ - return tuple of the data which the view requires for each course + Return a dict of the data which the view requires for each course """ - return ( - course.display_name, - reverse_course_url('course_handler', course.id), - get_lms_link_for_item(course.location), - _get_rerun_link_for_item(course.id), - course.display_org_with_default, - course.display_number_with_default, - course.location.run - ) + return { + 'display_name': course.display_name, + 'course_key': unicode(course.location.course_key), + 'url': reverse_course_url('course_handler', course.id), + 'lms_link': get_lms_link_for_item(course.location), + 'rerun_link': _get_rerun_link_for_item(course.id), + 'org': course.display_org_with_default, + 'number': course.display_number_with_default, + 'run': course.location.run + } def format_unsucceeded_course_for_view(uca): """ - return tuple of the data which the view requires for each unsucceeded course + Return a dict of the data which the view requires for each unsucceeded course """ - return ( - uca.display_name, - uca.course_key.org, - uca.course_key.course, - uca.course_key.run, - True if uca.state == CourseRerunUIStateManager.State.FAILED else False, - True if uca.state == CourseRerunUIStateManager.State.IN_PROGRESS else False, - reverse_course_url('course_notifications_handler', uca.course_key, kwargs={ - 'action_state_id': uca.id, - }) if uca.state == CourseRerunUIStateManager.State.FAILED else '' - ) + return { + 'display_name': uca.display_name, + 'course_key': unicode(uca.course_key), + 'org': uca.course_key.org, + 'number': uca.course_key.course, + 'run': uca.course_key.run, + 'is_failed': True if uca.state == CourseRerunUIStateManager.State.FAILED else False, + 'is_in_progress': True if uca.state == CourseRerunUIStateManager.State.IN_PROGRESS else False, + 'dismiss_link': + reverse_course_url('course_notifications_handler', uca.course_key, kwargs={ + 'action_state_id': uca.id, + }) if uca.state == CourseRerunUIStateManager.State.FAILED else '' + } # remove any courses in courses that are also in the unsucceeded_course_actions list unsucceeded_action_course_keys = [uca.course_key for uca in unsucceeded_course_actions] diff --git a/cms/templates/index.html b/cms/templates/index.html index b89d8c75fe..72b384f4bf 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -137,24 +137,24 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {

Courses Being Processed

    - %for display_name, org, num, run, state_failed, state_in_progress, dismiss_link in sorted(unsucceeded_course_actions, key=lambda s: s[0].lower() if s[0] is not None else ''): + %for course_info in sorted(unsucceeded_course_actions, key=lambda s: s['display_name'].lower() if s['display_name'] is not None else ''): - %if state_in_progress: -
  • + %if course_info['is_in_progress']: +
  • -

    ${display_name}

    +

    ${course_info['display_name']}

    - ${_("Organization:")} ${org} + ${_("Organization:")} ${course_info['org']} ${_("Course Number:")} - ${num} + ${course_info['number']} - ${_("Course Run:")} ${run} + ${_("Course Run:")} ${course_info['run']}
    @@ -177,22 +177,22 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) { - %if state_failed: -
  • + %if course_info['is_failed']: +
  • -

    ${display_name}

    +

    ${course_info['display_name']}

    - ${_("Organization:")} ${org} + ${_("Organization:")} ${course_info['org']} ${_("Course Number:")} - ${num} + ${course_info['number']} - ${_("Course Run:")} ${run} + ${_("Course Run:")} ${course_info['run']}
    @@ -211,7 +211,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {
    • - + ${_("Dismiss")} @@ -228,21 +228,21 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) { %if len(courses) > 0:
        - %for course, url, lms_link, rerun_link, org, num, run in sorted(courses, key=lambda s: s[0].lower() if s[0] is not None else ''): -
      • - -

        ${course}

        + %for course_info in sorted(courses, key=lambda s: s['display_name'].lower() if s['display_name'] is not None else ''): +
      • + +

        ${course_info['display_name']}

        - ${_("Organization:")} ${org} + ${_("Organization:")} ${course_info['org']} ${_("Course Number:")} - ${num} + ${course_info['number']} - ${_("Course Run:")} ${run} + ${_("Course Run:")} ${course_info['run']}
        @@ -250,11 +250,11 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {
        • diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index ca886da487..a8eacf20d1 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -10,6 +10,7 @@ bleach==1.4 html5lib==0.999 boto==2.13.3 celery==3.0.19 +cssselect==0.9.1 dealer==0.2.3 distribute>=0.6.28, <0.7 django-babel-underscore==0.1.0 From 2839ad871131ea96c5a8949c841c08e9c22a2131 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Tue, 19 Aug 2014 10:28:14 -0400 Subject: [PATCH 22/42] PR cleanup --- cms/djangoapps/contentstore/views/course.py | 3 ++- cms/static/js/index.js | 2 +- .../js/spec/views/pages/course_rerun_spec.js | 24 +++++++++---------- cms/static/js/spec/views/pages/index_spec.js | 5 ++-- cms/static/js/views/course_rerun.js | 4 ++-- cms/static/js/views/pages/course_outline.js | 2 +- .../migrations/0002_add_rerun_display_name.py | 2 +- 7 files changed, 21 insertions(+), 21 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 33f083a020..760db51924 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -240,6 +240,7 @@ def course_rerun_handler(request, course_key_string): GET html: return html page with form to rerun a course for the given course id """ + # Only global staff (PMs) are able to rerun courses during the soft launch if not GlobalStaff().has_user(request.user): raise PermissionDenied() course_key = CourseKey.from_string(course_key_string) @@ -407,7 +408,7 @@ def course_listing(request): def _get_rerun_link_for_item(course_key): - return '/course_rerun/{}/{}/{}'.format(course_key.org, course_key.course, course_key.run) + return reverse_course_url('course_rerun_handler', course_key) @login_required diff --git a/cms/static/js/index.js b/cms/static/js/index.js index de124f224b..91d8d71c96 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -7,7 +7,7 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], url: $(this).data('dismiss-link'), type: 'DELETE', success: function(result) { - window.location.reload() + window.location.reload(); } }); }; diff --git a/cms/static/js/spec/views/pages/course_rerun_spec.js b/cms/static/js/spec/views/pages/course_rerun_spec.js index 077c395313..fc77d29536 100644 --- a/cms/static/js/spec/views/pages/course_rerun_spec.js +++ b/cms/static/js/spec/views/pages/course_rerun_spec.js @@ -1,5 +1,5 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers", "js/views/course_rerun"], - function ($, create_sinon, view_helpers, CourseRerunPage) { + function ($, create_sinon, view_helpers, CourseRerunUtils) { describe("Create course rerun page", function () { var selectors = { courseOrg: '.rerun-course-org', @@ -30,7 +30,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" view_helpers.installMockAnalytics(); window.source_course_key = 'test_course_key'; appendSetFixtures(mockCreateCourseRerunHTML); - CourseRerunPage.onReady(); + CourseRerunUtils.onReady(); }); afterEach(function () { @@ -38,33 +38,33 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" delete window.source_course_key; }); - describe("validateRequiredField", function () { - it("has a message for an empty string", function () { - var message = CourseRerunPage.validateRequiredField(''); + describe("Field validation", function () { + it("returns a message for an empty string", function () { + var message = CourseRerunUtils.validateRequiredField(''); expect(message).not.toBe(''); }); - it("does not have a message for a non empty string", function () { - var message = CourseRerunPage.validateRequiredField('edX'); + it("does not return a message for a non empty string", function () { + var message = CourseRerunUtils.validateRequiredField('edX'); expect(message).toBe(''); }); }); - describe("setNewCourseFieldInErr", function () { + describe("Error messages", function () { var setErrorMessage = function(selector, message) { var element = $(selector).parent(); - CourseRerunPage.setNewCourseFieldInErr(element, message); + CourseRerunUtils.setNewCourseFieldInErr(element, message); return element; }; - it("can show an error message", function () { + it("shows an error message", function () { var element = setErrorMessage(selectors.courseOrg, 'error message'); expect(element).toHaveClass(classes.error); expect(element.children(selectors.errorField)).not.toHaveClass(classes.hidden); expect(element.children(selectors.errorField)).toContainText('error message'); }); - it("can hide an error message", function () { + it("hides an error message", function () { var element = setErrorMessage(selectors.courseOrg, ''); expect(element).not.toHaveClass(classes.error); expect(element.children(selectors.errorField)).toHaveClass(classes.hidden); @@ -93,7 +93,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" }); }); - it("can save course reruns", function () { + it("saves course reruns", function () { var requests = create_sinon.requests(this); window.source_course_key = 'test_course_key'; fillInFields('DemoX', 'DM101', '2014', 'Demo course'); diff --git a/cms/static/js/spec/views/pages/index_spec.js b/cms/static/js/spec/views/pages/index_spec.js index bffe8b5e3b..5eafbc476d 100644 --- a/cms/static/js/spec/views/pages/index_spec.js +++ b/cms/static/js/spec/views/pages/index_spec.js @@ -1,12 +1,12 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers", "js/index"], - function ($, create_sinon, view_helpers, IndexPage) { + function ($, create_sinon, view_helpers, IndexUtils) { describe("Course listing page", function () { var mockIndexPageHTML = readFixtures('mock/mock-index-page.underscore'); beforeEach(function () { view_helpers.installMockAnalytics(); appendSetFixtures(mockIndexPageHTML); - IndexPage.onReady(); + IndexUtils.onReady(); }); afterEach(function () { @@ -14,7 +14,6 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" delete window.source_course_key; }); - it("can dismiss notifications", function () { var requests = create_sinon.requests(this); $('.dismiss-button').click(); diff --git a/cms/static/js/views/course_rerun.js b/cms/static/js/views/course_rerun.js index 529e9029a9..e9860a0796 100644 --- a/cms/static/js/views/course_rerun.js +++ b/cms/static/js/views/course_rerun.js @@ -41,7 +41,7 @@ define(["domReady", "jquery", "underscore"], }, function (data) { if (data.url !== undefined) { - window.location = data.url + window.location = data.url; } else if (data.ErrMsg !== undefined) { $('.wrapper-error').addClass('is-shown').removeClass('is-hidden'); $('#course_rerun_error').html('

        ' + data.ErrMsg + '

        '); @@ -64,7 +64,7 @@ define(["domReady", "jquery", "underscore"], $('#course_rerun_error').html(''); $('wrapper-error').removeClass('is-shown').addClass('is-hidden'); $('.rerun-course-save').off('click'); - window.location.href = '/course/' + window.location.href = '/course/'; }; var validateRequiredField = function (msg) { diff --git a/cms/static/js/views/pages/course_outline.js b/cms/static/js/views/pages/course_outline.js index 464c9d5516..87ba45309e 100644 --- a/cms/static/js/views/pages/course_outline.js +++ b/cms/static/js/views/pages/course_outline.js @@ -25,7 +25,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views self.outlineView.handleAddEvent(event); }); this.model.on('change', this.setCollapseExpandVisibility, this); - $('.dismiss-button').bind('click', this.dismissNotification) + $('.dismiss-button').bind('click', this.dismissNotification); }, setCollapseExpandVisibility: function() { diff --git a/common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py b/common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py index 8710b96bae..fc02006aff 100644 --- a/common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py +++ b/common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py @@ -73,4 +73,4 @@ class Migration(SchemaMigration): } } - complete_apps = ['course_action_state'] \ No newline at end of file + complete_apps = ['course_action_state'] From 78879ebc182f63520b3eae20f276f971773937f2 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Tue, 19 Aug 2014 11:02:38 -0400 Subject: [PATCH 23/42] Asset urls must start w/ slash LMS-11233 --- cms/djangoapps/contentstore/utils.py | 2 +- cms/djangoapps/contentstore/views/assets.py | 10 ++------- common/djangoapps/static_replace/__init__.py | 2 +- .../xmodule/xmodule/contentstore/content.py | 22 ++++++++++++++----- .../lib/xmodule/xmodule/contentstore/mongo.py | 2 +- lms/djangoapps/courseware/courses.py | 2 +- 6 files changed, 22 insertions(+), 18 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index b3b9ed3730..f7eb22f99a 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -146,7 +146,7 @@ def get_lms_link_for_about_page(course_key): def course_image_url(course): """Returns the image url for the course.""" loc = StaticContent.compute_location(course.location.course_key, course.course_image) - path = loc.to_deprecated_string() + path = StaticContent.serialize_asset_key_with_slash(loc) return path diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index c4efaab860..e8924fe7c9 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -277,7 +277,7 @@ def _get_asset_json(display_name, date, location, thumbnail_location, locked): """ Helper method for formatting the asset information to send to client. """ - asset_url = _add_slash(location.to_deprecated_string()) + asset_url = StaticContent.serialize_asset_key_with_slash(location) external_url = settings.LMS_BASE + asset_url return { 'display_name': display_name, @@ -285,14 +285,8 @@ def _get_asset_json(display_name, date, location, thumbnail_location, locked): 'url': asset_url, 'external_url': external_url, 'portable_url': StaticContent.get_static_path_from_location(location), - 'thumbnail': _add_slash(unicode(thumbnail_location)) if thumbnail_location else None, + 'thumbnail': StaticContent.serialize_asset_key_with_slash(thumbnail_location) if thumbnail_location else None, 'locked': locked, # Needed for Backbone delete/update. 'id': unicode(location) } - - -def _add_slash(url): - if not url.startswith('/'): - url = '/' + url # TODO - re-address this once LMS-11198 is tackled. - return url diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index cd2e077444..8396940f8e 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -97,7 +97,7 @@ def replace_static_urls(text, data_directory, course_id=None, static_asset_path= Replace /static/$stuff urls either with their correct url as generated by collectstatic, (/static/$md5_hashed_stuff) or by the course-specific content static url /static/$course_data_dir/$stuff, or, if course_namespace is not None, by the - correct url in the contentstore (c4x://) + correct url in the contentstore (/c4x/.. or /asset-loc:..) text: The source text to do the substitution in data_directory: The directory in which course data is stored diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index c45a56f391..c3c653125f 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -64,9 +64,6 @@ class StaticContent(object): def get_id(self): return self.location - def get_url_path(self): - return self.location.to_deprecated_string() - @property def data(self): return self._data @@ -108,7 +105,9 @@ class StaticContent(object): assert(isinstance(course_key, CourseKey)) placeholder_id = uuid.uuid4().hex # create a dummy asset location with a fake but unique name. strip off the name, and return it - url_path = unicode(course_key.make_asset_key('asset', placeholder_id).for_branch(None)) + url_path = StaticContent.serialize_asset_key_with_slash( + course_key.make_asset_key('asset', placeholder_id).for_branch(None) + ) return url_path.replace(placeholder_id, '') @staticmethod @@ -133,7 +132,7 @@ class StaticContent(object): # Generate url of urlparse.path component scheme, netloc, orig_path, params, query, fragment = urlparse(path) loc = StaticContent.compute_location(course_id, orig_path) - loc_url = loc.to_deprecated_string() + loc_url = StaticContent.serialize_asset_key_with_slash(loc) # parse the query params for "^/static/" and replace with the location url orig_query = parse_qsl(query) @@ -144,7 +143,7 @@ class StaticContent(object): course_id, query_value[len('/static/'):], ) - new_query_url = new_query.to_deprecated_string() + new_query_url = StaticContent.serialize_asset_key_with_slash(new_query) new_query_list.append((query_name, new_query_url)) else: new_query_list.append((query_name, query_value)) @@ -155,6 +154,17 @@ class StaticContent(object): def stream_data(self): yield self._data + @staticmethod + def serialize_asset_key_with_slash(asset_key): + """ + Legacy code expects the serialized asset key to start w/ a slash; so, do that in one place + :param asset_key: + """ + url = unicode(asset_key) + if not url.startswith('/'): + url = '/' + url # TODO - re-address this once LMS-11198 is tackled. + return url + class StaticContentStream(StaticContent): def __init__(self, loc, name, content_type, stream, last_modified_at=None, thumbnail_location=None, import_path=None, diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index 5a04bf91ee..7d8bf949ad 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/common/lib/xmodule/xmodule/contentstore/mongo.py @@ -66,7 +66,7 @@ class MongoContentStore(ContentStore): self.delete(content_id) # delete is a noop if the entry doesn't exist; so, don't waste time checking thumbnail_location = content.thumbnail_location.to_deprecated_list_repr() if content.thumbnail_location else None - with self.fs.new_file(_id=content_id, filename=content.get_url_path(), content_type=content.content_type, + with self.fs.new_file(_id=content_id, filename=unicode(content.location), content_type=content.content_type, displayname=content.name, content_son=content_son, thumbnail_location=thumbnail_location, import_path=content.import_path, diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 724efc1989..6286a5c31b 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -118,7 +118,7 @@ def course_image_url(course): url += '/images/course_image.jpg' else: loc = StaticContent.compute_location(course.id, course.course_image) - url = loc.to_deprecated_string() + url = StaticContent.serialize_asset_key_with_slash(loc) return url From 63603d70f4446276ae43113fa8cbd3ac711ec39d Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Tue, 19 Aug 2014 15:52:50 -0400 Subject: [PATCH 24/42] Refactor shared course creation validation into create_course_utils --- cms/static/js/index.js | 152 ++++----------- .../js/spec/views/pages/course_rerun_spec.js | 99 +++++----- cms/static/js/views/course_rerun.js | 176 ++++-------------- .../js/views/utils/create_course_utils.js | 151 +++++++++++++++ 4 files changed, 276 insertions(+), 302 deletions(-) create mode 100644 cms/static/js/views/utils/create_course_utils.js diff --git a/cms/static/js/index.js b/cms/static/js/index.js index 91d8d71c96..e6bbf124c7 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -1,5 +1,23 @@ -define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], - function (domReady, $, _, CancelOnEscape) { +define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/views/utils/create_course_utils"], + function (domReady, $, _, CancelOnEscape, CreateCourseUtilsFactory) { + var CreateCourseUtils = CreateCourseUtilsFactory({ + name: '.new-course-name', + org: '.new-course-org', + number: '.new-course-number', + run: '.new-course-run', + save: '.new-course-save', + errorWrapper: '.wrap-error', + errorMessage: '#course_creation_error', + tipError: 'span.tip-error', + error: '.error', + allowUnicode: '.allow-unicode-course-id' + }, { + shown: 'is-shown', + showing: 'is-showing', + hiding: 'is-hiding', + disabled: 'is-disabled', + error: 'error' + }); var dismissNotification = function (e) { e.preventDefault(); @@ -14,19 +32,7 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], var saveNewCourse = function (e) { e.preventDefault(); - // One final check for empty values - var errors = _.reduce( - ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], - function (acc, ele) { - var $ele = $(ele); - var error = validateRequiredField($ele.val()); - setNewCourseFieldInErr($ele.parent('li'), error); - return error ? true : acc; - }, - false - ); - - if (errors) { + if (CreateCourseUtils.hasInvalidRequiredFields()) { return; } @@ -36,29 +42,19 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], var number = $newCourseForm.find('.new-course-number').val(); var run = $newCourseForm.find('.new-course-run').val(); - analytics.track('Created a Course', { - 'org': org, - 'number': number, - 'display_name': display_name, - 'run': run - }); + course_info = { + org: org, + number: number, + display_name: display_name, + run: run + }; - $.postJSON('/course/', { - 'org': org, - 'number': number, - 'display_name': display_name, - 'run': run - }, - function (data) { - if (data.url !== undefined) { - window.location = data.url; - } else if (data.ErrMsg !== undefined) { - $('.wrap-error').addClass('is-shown'); - $('#course_creation_error').html('

        ' + data.ErrMsg + '

        '); - $('.new-course-save').addClass('is-disabled'); - } - } - ); + analytics.track('Created a Course', course_info); + CreateCourseUtils.createCourse(course_info, function (errorMessage) { + $('.wrap-error').addClass('is-shown'); + $('#course_creation_error').html('

        ' + errorMessage + '

        '); + $('.new-course-save').addClass('is-disabled'); + }); }; var cancelNewCourse = function (e) { @@ -77,25 +73,6 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], $('.new-course-save').off('click'); }; - // Check that a course (org, number, run) doesn't use any special characters - var validateCourseItemEncoding = function (item) { - var required = validateRequiredField(item); - if (required) { - return required; - } - if ($('.allow-unicode-course-id').val() === 'True'){ - if (/\s/g.test(item)) { - return gettext('Please do not use any spaces in this field.'); - } - } - else{ - if (item !== encodeURIComponent(item)) { - return gettext('Please do not use any spaces or special characters in this field.'); - } - } - return ''; - }; - var addNewCourse = function (e) { e.preventDefault(); $('.new-course-button').addClass('is-disabled'); @@ -108,68 +85,7 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], $cancelButton.bind('click', cancelNewCourse); CancelOnEscape($cancelButton); - // Ensure that org/course_num/run < 65 chars. - var validateTotalCourseItemsLength = function () { - var totalLength = _.reduce( - ['.new-course-org', '.new-course-number', '.new-course-run'], - function (sum, ele) { - return sum + $(ele).val().length; - }, 0 - ); - if (totalLength > 65) { - $('.wrap-error').addClass('is-shown'); - $('#course_creation_error').html('

        ' + gettext('The combined length of the organization, course number, and course run fields cannot be more than 65 characters.') + '

        '); - $('.new-course-save').addClass('is-disabled'); - } - else { - $('.wrap-error').removeClass('is-shown'); - } - }; - - // Handle validation asynchronously - _.each( - ['.new-course-org', '.new-course-number', '.new-course-run'], - function (ele) { - var $ele = $(ele); - $ele.on('keyup', function (event) { - // Don't bother showing "required field" error when - // the user tabs into a new field; this is distracting - // and unnecessary - if (event.keyCode === 9) { - return; - } - var error = validateCourseItemEncoding($ele.val()); - setNewCourseFieldInErr($ele.parent('li'), error); - validateTotalCourseItemsLength(); - }); - } - ); - var $name = $('.new-course-name'); - $name.on('keyup', function () { - var error = validateRequiredField($name.val()); - setNewCourseFieldInErr($name.parent('li'), error); - validateTotalCourseItemsLength(); - }); - }; - - var validateRequiredField = function (msg) { - return msg.length === 0 ? gettext('Required field.') : ''; - }; - - var setNewCourseFieldInErr = function (el, msg) { - if(msg) { - el.addClass('error'); - el.children('span.tip-error').addClass('is-showing').removeClass('is-hiding').text(msg); - $('.new-course-save').addClass('is-disabled'); - } - else { - el.removeClass('error'); - el.children('span.tip-error').addClass('is-hiding').removeClass('is-showing'); - // One "error" div is always present, but hidden or shown - if($('.error').length === 1) { - $('.new-course-save').removeClass('is-disabled'); - } - } + CreateCourseUtils.configureHandlers(); }; var onReady = function () { diff --git a/cms/static/js/spec/views/pages/course_rerun_spec.js b/cms/static/js/spec/views/pages/course_rerun_spec.js index fc77d29536..8bd31809c3 100644 --- a/cms/static/js/spec/views/pages/course_rerun_spec.js +++ b/cms/static/js/spec/views/pages/course_rerun_spec.js @@ -1,17 +1,24 @@ -define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers", "js/views/course_rerun"], - function ($, create_sinon, view_helpers, CourseRerunUtils) { +define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers", "js/views/course_rerun", + "js/views/utils/create_course_utils"], + function ($, create_sinon, view_helpers, CourseRerunUtils, CreateCourseUtilsFactory) { describe("Create course rerun page", function () { var selectors = { - courseOrg: '.rerun-course-org', - courseNumber: '.rerun-course-number', - courseRun: '.rerun-course-run', - courseName: '.rerun-course-name', - errorField: '.tip-error', - saveButton: '.rerun-course-save', - cancelButton: '.rerun-course-cancel', - errorMessage: '.wrapper-error' + org: '.rerun-course-org', + number: '.rerun-course-number', + run: '.rerun-course-run', + name: '.rerun-course-name', + tipError: 'span.tip-error', + save: '.rerun-course-save', + cancel: '.rerun-course-cancel', + errorWrapper: '.wrapper-error', + errorMessage: '#course_rerun_error', + error: '.error', + allowUnicode: '.allow-unicode-course-id' }, classes = { + shown: 'is-shown', + showing: 'is-showing', + hiding: 'is-hidden', hidden: 'is-hidden', error: 'error', disabled: 'is-disabled', @@ -19,11 +26,13 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" }, mockCreateCourseRerunHTML = readFixtures('mock/mock-create-course-rerun.underscore'); + var CreateCourseUtils = CreateCourseUtilsFactory(selectors, classes); + var fillInFields = function (org, number, run, name) { - $(selectors.courseOrg).val(org); - $(selectors.courseNumber).val(number); - $(selectors.courseRun).val(run); - $(selectors.courseName).val(name); + $(selectors.org).val(org); + $(selectors.number).val(number); + $(selectors.run).val(run); + $(selectors.name).val(name); }; beforeEach(function () { @@ -40,12 +49,12 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" describe("Field validation", function () { it("returns a message for an empty string", function () { - var message = CourseRerunUtils.validateRequiredField(''); + var message = CreateCourseUtils.validateRequiredField(''); expect(message).not.toBe(''); }); it("does not return a message for a non empty string", function () { - var message = CourseRerunUtils.validateRequiredField('edX'); + var message = CreateCourseUtils.validateRequiredField('edX'); expect(message).toBe(''); }); }); @@ -53,43 +62,43 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" describe("Error messages", function () { var setErrorMessage = function(selector, message) { var element = $(selector).parent(); - CourseRerunUtils.setNewCourseFieldInErr(element, message); + CreateCourseUtils.setNewCourseFieldInErr(element, message); return element; }; it("shows an error message", function () { - var element = setErrorMessage(selectors.courseOrg, 'error message'); + var element = setErrorMessage(selectors.org, 'error message'); expect(element).toHaveClass(classes.error); - expect(element.children(selectors.errorField)).not.toHaveClass(classes.hidden); - expect(element.children(selectors.errorField)).toContainText('error message'); + expect(element.children(selectors.tipError)).not.toHaveClass(classes.hidden); + expect(element.children(selectors.tipError)).toContainText('error message'); }); it("hides an error message", function () { - var element = setErrorMessage(selectors.courseOrg, ''); + var element = setErrorMessage(selectors.org, ''); expect(element).not.toHaveClass(classes.error); - expect(element.children(selectors.errorField)).toHaveClass(classes.hidden); + expect(element.children(selectors.tipError)).toHaveClass(classes.hidden); }); it("disables the save button", function () { - setErrorMessage(selectors.courseOrg, 'error message'); - expect($(selectors.saveButton)).toHaveClass(classes.disabled); + setErrorMessage(selectors.org, 'error message'); + expect($(selectors.save)).toHaveClass(classes.disabled); }); it("enables the save button when all errors are removed", function () { - setErrorMessage(selectors.courseOrg, 'error message 1'); - setErrorMessage(selectors.courseNumber, 'error message 2'); - expect($(selectors.saveButton)).toHaveClass(classes.disabled); - setErrorMessage(selectors.courseOrg, ''); - setErrorMessage(selectors.courseNumber, ''); - expect($(selectors.saveButton)).not.toHaveClass(classes.disabled); + setErrorMessage(selectors.org, 'error message 1'); + setErrorMessage(selectors.number, 'error message 2'); + expect($(selectors.save)).toHaveClass(classes.disabled); + setErrorMessage(selectors.org, ''); + setErrorMessage(selectors.number, ''); + expect($(selectors.save)).not.toHaveClass(classes.disabled); }); it("does not enable the save button when errors remain", function () { - setErrorMessage(selectors.courseOrg, 'error message 1'); - setErrorMessage(selectors.courseNumber, 'error message 2'); - expect($(selectors.saveButton)).toHaveClass(classes.disabled); - setErrorMessage(selectors.courseOrg, ''); - expect($(selectors.saveButton)).toHaveClass(classes.disabled); + setErrorMessage(selectors.org, 'error message 1'); + setErrorMessage(selectors.number, 'error message 2'); + expect($(selectors.save)).toHaveClass(classes.disabled); + setErrorMessage(selectors.org, ''); + expect($(selectors.save)).toHaveClass(classes.disabled); }); }); @@ -97,7 +106,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" var requests = create_sinon.requests(this); window.source_course_key = 'test_course_key'; fillInFields('DemoX', 'DM101', '2014', 'Demo course'); - $(selectors.saveButton).click(); + $(selectors.save).click(); create_sinon.expectJsonRequest(requests, 'POST', '/course/', { source_course_key: 'test_course_key', org: 'DemoX', @@ -105,28 +114,28 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" run: '2014', display_name: 'Demo course' }); - expect($(selectors.saveButton)).toHaveClass(classes.disabled); - expect($(selectors.saveButton)).toHaveClass(classes.processing); - expect($(selectors.cancelButton)).toHaveClass(classes.hidden); + expect($(selectors.save)).toHaveClass(classes.disabled); + expect($(selectors.save)).toHaveClass(classes.processing); + expect($(selectors.cancel)).toHaveClass(classes.hidden); }); it("displays an error when saving fails", function () { var requests = create_sinon.requests(this); fillInFields('DemoX', 'DM101', '2014', 'Demo course'); - $(selectors.saveButton).click(); + $(selectors.save).click(); create_sinon.respondWithJson(requests, { ErrMsg: 'error message' }); - expect($(selectors.errorMessage)).not.toHaveClass(classes.hidden); - expect($(selectors.errorMessage)).toContainText('error message'); - expect($(selectors.saveButton)).not.toHaveClass(classes.processing); - expect($(selectors.cancelButton)).not.toHaveClass(classes.hidden); + expect($(selectors.errorWrapper)).not.toHaveClass(classes.hidden); + expect($(selectors.errorWrapper)).toContainText('error message'); + expect($(selectors.save)).not.toHaveClass(classes.processing); + expect($(selectors.cancel)).not.toHaveClass(classes.hidden); }); it("does not save if there are validation errors", function () { var requests = create_sinon.requests(this); fillInFields('DemoX', 'DM101', '', 'Demo course'); - $(selectors.saveButton).click(); + $(selectors.save).click(); expect(requests.length).toBe(0); }); }); diff --git a/cms/static/js/views/course_rerun.js b/cms/static/js/views/course_rerun.js index e9860a0796..60fd88fd7d 100644 --- a/cms/static/js/views/course_rerun.js +++ b/cms/static/js/views/course_rerun.js @@ -1,21 +1,28 @@ -define(["domReady", "jquery", "underscore"], - function (domReady, $, _) { +define(["domReady", "jquery", "underscore", "js/views/utils/create_course_utils"], + function (domReady, $, _, CreateCourseUtilsFactory) { + var CreateCourseUtils = CreateCourseUtilsFactory({ + name: '.rerun-course-name', + org: '.rerun-course-org', + number: '.rerun-course-number', + run: '.rerun-course-run', + save: '.rerun-course-save', + errorWrapper: '.wrapper-error', + errorMessage: '#course_rerun_error', + tipError: 'span.tip-error', + error: '.error', + allowUnicode: '.allow-unicode-course-id' + }, { + shown: 'is-shown', + showing: 'is-showing', + hiding: 'is-hidden', + disabled: 'is-disabled', + error: 'error' + }); var saveRerunCourse = function (e) { e.preventDefault(); - // One final check for errors - var errors = _.reduce( - ['.rerun-course-name', '.rerun-course-org', '.rerun-course-number', '.rerun-course-run'], - function (acc, ele) { - var $ele = $(ele); - var error = validateRequiredField($ele.val()); - setNewCourseFieldInErr($ele.parent('li'), error); - return error ? true : acc; - }, - false - ); - if (errors) { + if (CreateCourseUtils.hasInvalidRequiredFields()) { return; } @@ -25,31 +32,22 @@ define(["domReady", "jquery", "underscore"], var number = $newCourseForm.find('.rerun-course-number').val(); var run = $newCourseForm.find('.rerun-course-run').val(); - analytics.track('Reran a Course', { - 'source_course_key': source_course_key, - 'org': org, - 'number': number, - 'display_name': display_name, - 'run': run + course_info = { + source_course_key: source_course_key, + org: org, + number: number, + display_name: display_name, + run: run + }; + + analytics.track('Reran a Course', course_info); + CreateCourseUtils.createCourse(course_info, function (errorMessage) { + $('.wrapper-error').addClass('is-shown').removeClass('is-hidden'); + $('#course_rerun_error').html('

        ' + errorMessage + '

        '); + $('.rerun-course-save').addClass('is-disabled').removeClass('is-processing').html(gettext('Create Re-run')); + $('.action-cancel').removeClass('is-hidden'); }); - $.postJSON('/course/', { - 'source_course_key': source_course_key, - 'org': org, - 'number': number, - 'display_name': display_name, - 'run': run - }, - function (data) { - if (data.url !== undefined) { - window.location = data.url; - } else if (data.ErrMsg !== undefined) { - $('.wrapper-error').addClass('is-shown').removeClass('is-hidden'); - $('#course_rerun_error').html('

        ' + data.ErrMsg + '

        '); - $('.rerun-course-save').addClass('is-disabled').removeClass('is-processing').html(gettext('Create Re-run')); - $('.action-cancel').removeClass('is-hidden'); - } - } - ); + // Go into creating re-run state $('.rerun-course-save').addClass('is-disabled').addClass('is-processing').html( '' + gettext('Processing Re-run Request') @@ -67,26 +65,6 @@ define(["domReady", "jquery", "underscore"], window.location.href = '/course/'; }; - var validateRequiredField = function (msg) { - return msg.length === 0 ? gettext('Required field.') : ''; - }; - - var setNewCourseFieldInErr = function (el, msg) { - if(msg) { - el.addClass('error'); - el.children('span.tip-error').addClass('is-shown').removeClass('is-hidden').text(msg); - $('.rerun-course-save').addClass('is-disabled'); - } - else { - el.removeClass('error'); - el.children('span.tip-error').addClass('is-hidden').removeClass('is-shown'); - // One "error" div is always present, but hidden or shown - if($('.error').length === 1) { - $('.rerun-course-save').removeClass('is-disabled'); - } - } - }; - var onReady = function () { var $cancelButton = $('.rerun-course-cancel'); var $courseRun = $('.rerun-course-run'); @@ -95,85 +73,7 @@ define(["domReady", "jquery", "underscore"], $cancelButton.bind('click', cancelRerunCourse); $('.cancel-button').bind('click', cancelRerunCourse); - // Check that a course (org, number, run) doesn't use any special characters - var validateCourseItemEncoding = function (item) { - var required = validateRequiredField(item); - if (required) { - return required; - } - if ($('.allow-unicode-course-id').val() === 'True'){ - if (/\s/g.test(item)) { - return gettext('Please do not use any spaces in this field.'); - } - } - else{ - if (item !== encodeURIComponent(item)) { - return gettext('Please do not use any spaces or special characters in this field.'); - } - } - return ''; - }; - - // Ensure that org/course_num/run < 65 chars. - var validateTotalCourseItemsLength = function () { - var totalLength = _.reduce( - ['.rerun-course-org', '.rerun-course-number', '.rerun-course-run'], - function (sum, ele) { - return sum + $(ele).val().length; - }, 0 - ); - if (totalLength > 65) { - $('.wrap-error').addClass('is-shown'); - $('#course_creation_error').html('

        ' + gettext('The combined length of the organization, course number, and course run fields cannot be more than 65 characters.') + '

        '); - $('.rerun-course-save').addClass('is-disabled'); - } - else { - $('.wrap-error').removeClass('is-shown'); - } - }; - - // Ensure that all fields are not empty - var validateFilledFields = function () { - return _.reduce( - ['.rerun-course-org', '.rerun-course-number', '.rerun-course-run', '.rerun-course-name'], - function (acc, ele) { - var $ele = $(ele); - return $ele.val().length !== 0 ? acc : false; - }, - true - ); - }; - - // Handle validation asynchronously - _.each( - ['.rerun-course-org', '.rerun-course-number', '.rerun-course-run'], - function (ele) { - var $ele = $(ele); - $ele.on('keyup', function (event) { - // Don't bother showing "required field" error when - // the user tabs into a new field; this is distracting - // and unnecessary - if (event.keyCode === 9) { - return; - } - var error = validateCourseItemEncoding($ele.val()); - setNewCourseFieldInErr($ele.parent(), error); - validateTotalCourseItemsLength(); - if(!validateFilledFields()) { - $('.rerun-course-save').addClass('is-disabled'); - } - }); - } - ); - var $name = $('.rerun-course-name'); - $name.on('keyup', function () { - var error = validateRequiredField($name.val()); - setNewCourseFieldInErr($name.parent(), error); - validateTotalCourseItemsLength(); - if(!validateFilledFields()) { - $('.rerun-course-save').addClass('is-disabled'); - } - }); + CreateCourseUtils.configureHandlers(); }; domReady(onReady); @@ -182,8 +82,6 @@ define(["domReady", "jquery", "underscore"], return { saveRerunCourse: saveRerunCourse, cancelRerunCourse: cancelRerunCourse, - validateRequiredField: validateRequiredField, - setNewCourseFieldInErr: setNewCourseFieldInErr, onReady: onReady }; }); diff --git a/cms/static/js/views/utils/create_course_utils.js b/cms/static/js/views/utils/create_course_utils.js new file mode 100644 index 0000000000..a8959fa247 --- /dev/null +++ b/cms/static/js/views/utils/create_course_utils.js @@ -0,0 +1,151 @@ +/** + * Provides utilities for validating courses during creation, for both new courses and reruns. + */ +define(["jquery", "underscore", "gettext"], + function ($, _, gettext) { + return function (selectors, classes) { + var validateRequiredField, validateCourseItemEncoding, validateTotalCourseItemsLength, setNewCourseFieldInErr, + hasInvalidRequiredFields, createCourse, validateFilledFields, configureHandlers; + + validateRequiredField = function (msg) { + return msg.length === 0 ? gettext('Required field.') : ''; + }; + + // Check that a course (org, number, run) doesn't use any special characters + validateCourseItemEncoding = function (item) { + var required = validateRequiredField(item); + if (required) { + return required; + } + if ($(selectors.allowUnicode).val() === 'True') { + if (/\s/g.test(item)) { + return gettext('Please do not use any spaces in this field.'); + } + } + else { + if (item !== encodeURIComponent(item)) { + return gettext('Please do not use any spaces or special characters in this field.'); + } + } + return ''; + }; + + // Ensure that org/course_num/run < 65 chars. + validateTotalCourseItemsLength = function () { + var totalLength = _.reduce( + [selectors.org, selectors.number, selectors.run], + function (sum, ele) { + return sum + $(ele).val().length; + }, 0 + ); + if (totalLength > 65) { + $(selectors.errorWrapper).addClass(classes.shown); + $(selectors.errorMessage).html('

        ' + gettext('The combined length of the organization, course number, and course run fields cannot be more than 65 characters.') + '

        '); + $(selectors.save).addClass(classes.disabled); + } + else { + $(selectors.errorWrapper).removeClass(classes.shown); + } + }; + + setNewCourseFieldInErr = function (el, msg) { + if (msg) { + el.addClass(classes.error); + el.children(selectors.tipError).addClass(classes.showing).removeClass(classes.hiding).text(msg); + $(selectors.save).addClass(classes.disabled); + } + else { + el.removeClass(classes.error); + el.children(selectors.tipError).addClass(classes.hiding).removeClass(classes.showing); + // One "error" div is always present, but hidden or shown + if ($(selectors.error).length === 1) { + $(selectors.save).removeClass(classes.disabled); + } + } + }; + + // One final check for empty values + hasInvalidRequiredFields = function () { + return _.reduce( + [selectors.name, selectors.org, selectors.number, selectors.run], + function (acc, ele) { + var $ele = $(ele); + var error = validateRequiredField($ele.val()); + setNewCourseFieldInErr($ele.parent(), error); + return error ? true : acc; + }, + false + ); + }; + + createCourse = function (courseInfo, errorHandler) { + $.postJSON( + '/course/', + courseInfo, + function (data) { + if (data.url !== undefined) { + window.location = data.url; + } else if (data.ErrMsg !== undefined) { + errorHandler(data.ErrMsg); + } + } + ); + }; + + // Ensure that all fields are not empty + validateFilledFields = function () { + return _.reduce( + [selectors.org, selectors.number, selectors.run, selectors.name], + function (acc, ele) { + var $ele = $(ele); + return $ele.val().length !== 0 ? acc : false; + }, + true + ); + }; + + // Handle validation asynchronously + configureHandlers = function () { + _.each( + [selectors.org, selectors.number, selectors.run], + function (ele) { + var $ele = $(ele); + $ele.on('keyup', function (event) { + // Don't bother showing "required field" error when + // the user tabs into a new field; this is distracting + // and unnecessary + if (event.keyCode === 9) { + return; + } + var error = validateCourseItemEncoding($ele.val()); + setNewCourseFieldInErr($ele.parent(), error); + validateTotalCourseItemsLength(); + if (!validateFilledFields()) { + $(selectors.save).addClass(classes.disabled); + } + }); + } + ); + var $name = $(selectors.name); + $name.on('keyup', function () { + var error = validateRequiredField($name.val()); + setNewCourseFieldInErr($name.parent(), error); + validateTotalCourseItemsLength(); + if (!validateFilledFields()) { + $(selectors.save).addClass(classes.disabled); + } + }); + }; + + return { + validateRequiredField: validateRequiredField, + validateCourseItemEncoding: validateCourseItemEncoding, + validateTotalCourseItemsLength: validateTotalCourseItemsLength, + setNewCourseFieldInErr: setNewCourseFieldInErr, + hasInvalidRequiredFields: hasInvalidRequiredFields, + createCourse: createCourse, + validateFilledFields: validateFilledFields, + configureHandlers: configureHandlers + }; + }; + }); From 24cdd7a4818bb00adba10c0c97193940eb7c8b98 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Tue, 19 Aug 2014 15:58:45 -0400 Subject: [PATCH 25/42] Assume any non-xml modulestore is r/w LMS-11235 --- common/djangoapps/xmodule_modifiers.py | 2 +- lms/djangoapps/courseware/courses.py | 9 +++++---- lms/djangoapps/courseware/views.py | 10 +++++----- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 8381c067b5..5f79014847 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -185,7 +185,7 @@ def add_staff_markup(user, has_instructor_access, block, view, frag, context): # TODO: make this more general, eg use an XModule attribute instead if isinstance(block, VerticalModule) and (not context or not context.get('child_of_vertical', False)): # check that the course is a mongo backed Studio course before doing work - is_mongo_course = modulestore().get_modulestore_type(block.location.course_key) == ModuleStoreEnum.Type.mongo + is_mongo_course = modulestore().get_modulestore_type(block.location.course_key) != ModuleStoreEnum.Type.xml is_studio_course = block.course_edit_method == "Studio" if is_studio_course and is_mongo_course: diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 6286a5c31b..dee09c0d59 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -360,14 +360,15 @@ def get_cms_block_link(block, page): return u"//{}/{}/{}".format(settings.CMS_BASE, page, block.location) -def get_studio_url(course_key, page): +def get_studio_url(course, page): """ Get the Studio URL of the page that is passed in. + + Args: + course (CourseDescriptor) """ - assert(isinstance(course_key, CourseKey)) - course = get_course_by_id(course_key) is_studio_course = course.course_edit_method == "Studio" - is_mongo_course = modulestore().get_modulestore_type(course_key) == ModuleStoreEnum.Type.mongo + is_mongo_course = modulestore().get_modulestore_type(course.id) != ModuleStoreEnum.Type.xml studio_link = None if is_studio_course and is_mongo_course: studio_link = get_cms_course_link(course, page) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 350485f2e3..66232198be 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -311,7 +311,7 @@ def index(request, course_id, chapter=None, section=None, u' far, should have gotten a course module for this user') return redirect(reverse('about_course', args=[course_key.to_deprecated_string()])) - studio_url = get_studio_url(course_key, 'course') + studio_url = get_studio_url(course, 'course') context = { 'csrf': csrf(request)['csrf_token'], @@ -419,7 +419,7 @@ def index(request, course_id, chapter=None, section=None, context['section_title'] = section_descriptor.display_name_with_default else: # section is none, so display a message - studio_url = get_studio_url(course_key, 'course') + studio_url = get_studio_url(course, 'course') prev_section = get_current_child(chapter_module) if prev_section is None: # Something went wrong -- perhaps this chapter has no sections visible to the user @@ -553,7 +553,7 @@ def course_info(request, course_id): staff_access = has_access(request.user, 'staff', course) masq = setup_masquerade(request, staff_access) # allow staff to toggle masquerade on info page reverifications = fetch_reverify_banner_info(request, course_key) - studio_url = get_studio_url(course_key, 'course_info') + studio_url = get_studio_url(course, 'course_info') context = { 'request': request, @@ -655,7 +655,7 @@ def course_about(request, course_id): course = get_course_with_access(request.user, 'see_exists', course_key) registered = registered_for_course(course, request.user) staff_access = has_access(request.user, 'staff', course) - studio_url = get_studio_url(course_key, 'settings/details') + studio_url = get_studio_url(course, 'settings/details') if has_access(request.user, 'load', course): course_target = reverse('info', args=[course.id.to_deprecated_string()]) @@ -812,7 +812,7 @@ def _progress(request, course_key, student_id): student = User.objects.prefetch_related("groups").get(id=student.id) courseware_summary = grades.progress_summary(student, request, course) - studio_url = get_studio_url(course_key, 'settings/grading') + studio_url = get_studio_url(course, 'settings/grading') grade_summary = grades.grade(student, request, course) if courseware_summary is None: From d1ffecf31f6d58c72f9f9956c4a126be7d81bb0b Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Tue, 19 Aug 2014 16:07:05 -0400 Subject: [PATCH 26/42] Fix lms wiki url parsing LMS-11240 --- lms/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/urls.py b/lms/urls.py index d3d220c289..f363bd2a74 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -180,7 +180,7 @@ if settings.WIKI_ENABLED: # never be returned by a reverse() so they come after the other url patterns url(r'^courses/{}/course_wiki/?$'.format(settings.COURSE_ID_PATTERN), 'course_wiki.views.course_wiki_redirect', name="course_wiki"), - url(r'^courses/(?:[^/]+/[^/]+/[^/]+)/wiki/', include(wiki_pattern())), + url(r'^courses/{}/wiki/'.format(settings.COURSE_ID_PATTERN), include(wiki_pattern())), ) if settings.COURSEWARE_ENABLED: From 1778c157034b86a5771059e2535a26417acf457b Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Tue, 19 Aug 2014 16:42:50 -0400 Subject: [PATCH 27/42] Refactor dismissNotification uses into shared logic in view_utils --- cms/static/js/index.js | 20 ++++++------------- cms/static/js/views/overview.js | 19 +++++------------- cms/static/js/views/pages/course_outline.js | 22 +++++---------------- cms/static/js/views/utils/view_utils.js | 18 ++++++++++++++++- 4 files changed, 33 insertions(+), 46 deletions(-) diff --git a/cms/static/js/index.js b/cms/static/js/index.js index e6bbf124c7..26206859eb 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -1,5 +1,6 @@ -define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/views/utils/create_course_utils"], - function (domReady, $, _, CancelOnEscape, CreateCourseUtilsFactory) { +define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/views/utils/create_course_utils", + "js/views/utils/view_utils"], + function (domReady, $, _, CancelOnEscape, CreateCourseUtilsFactory, ViewUtils) { var CreateCourseUtils = CreateCourseUtilsFactory({ name: '.new-course-name', org: '.new-course-org', @@ -19,16 +20,6 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie error: 'error' }); - var dismissNotification = function (e) { - e.preventDefault(); - $.ajax({ - url: $(this).data('dismiss-link'), - type: 'DELETE', - success: function(result) { - window.location.reload(); - } - }); - }; var saveNewCourse = function (e) { e.preventDefault(); @@ -90,13 +81,14 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie var onReady = function () { $('.new-course-button').bind('click', addNewCourse); - $('.dismiss-button').bind('click', dismissNotification); + $('.dismiss-button').bind('click', ViewUtils.deleteNotificationHandler(function () { + window.location.reload(); + })); }; domReady(onReady); return { - dismissNotification: dismissNotification, onReady: onReady }; }); diff --git a/cms/static/js/views/overview.js b/cms/static/js/views/overview.js index 8ceaa40681..f83d46ee9e 100644 --- a/cms/static/js/views/overview.js +++ b/cms/static/js/views/overview.js @@ -1,21 +1,10 @@ define(["domReady", "jquery", "jquery.ui", "underscore", "gettext", "js/views/feedback_notification", - "js/utils/cancel_on_escape", "js/utils/date_utils", "js/utils/module"], + "js/utils/cancel_on_escape", "js/utils/date_utils", "js/utils/module", "js/views/utils/view_utils"], function (domReady, $, ui, _, gettext, NotificationView, CancelOnEscape, - DateUtils, ModuleUtils) { + DateUtils, ModuleUtils, ViewUtils) { var modalSelector = '.edit-section-publish-settings'; - var dismissNotification = function (e) { - e.preventDefault(); - $.ajax({ - url: $('.dismiss-button').data('dismiss-link'), - type: 'GET', - success: function(result) { - $('.wrapper-alert-announcement').remove() - } - }); - }; - var toggleSections = function(e) { e.preventDefault(); @@ -233,7 +222,9 @@ define(["domReady", "jquery", "jquery.ui", "underscore", "gettext", "js/views/fe $('.toggle-button-sections').bind('click', toggleSections); $('.expand-collapse').bind('click', toggleSubmodules); - $('.dismiss-button').bind('click', dismissNotification); + $('.dismiss-button').bind('click', ViewUtils.deleteNotificationHandler(function () { + $('.wrapper-alert-announcement').remove(); + })); var $body = $('body'); $body.on('click', '.section-published-date .edit-release-date', editSectionPublishDate); diff --git a/cms/static/js/views/pages/course_outline.js b/cms/static/js/views/pages/course_outline.js index 87ba45309e..a812eb7b02 100644 --- a/cms/static/js/views/pages/course_outline.js +++ b/cms/static/js/views/pages/course_outline.js @@ -2,8 +2,8 @@ * This page is used to show the user an outline of the course. */ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views/utils/xblock_utils", - "js/views/course_outline"], - function ($, _, gettext, BasePage, XBlockViewUtils, CourseOutlineView) { + "js/views/course_outline", "js/views/utils/view_utils"], + function ($, _, gettext, BasePage, XBlockViewUtils, CourseOutlineView, ViewUtils) { var expandedLocators, CourseOutlinePage; CourseOutlinePage = BasePage.extend({ @@ -25,7 +25,9 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views self.outlineView.handleAddEvent(event); }); this.model.on('change', this.setCollapseExpandVisibility, this); - $('.dismiss-button').bind('click', this.dismissNotification); + $('.dismiss-button').bind('click', ViewUtils.deleteNotificationHandler(function () { + $('.wrapper-alert-announcement').removeClass('is-shown').addClass('is-hidden') + })); }, setCollapseExpandVisibility: function() { @@ -98,20 +100,6 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views } }, this); } - }, - - /** - * Dismiss the course rerun notification. - */ - dismissNotification: function (e) { - e.preventDefault(); - $.ajax({ - url: $('.dismiss-button').data('dismiss-link'), - type: 'DELETE', - success: function(result) { - $('.wrapper-alert-announcement').removeClass('is-shown').addClass('is-hidden') - } - }); } }); diff --git a/cms/static/js/views/utils/view_utils.js b/cms/static/js/views/utils/view_utils.js index 98eab24d96..9719ea9247 100644 --- a/cms/static/js/views/utils/view_utils.js +++ b/cms/static/js/views/utils/view_utils.js @@ -5,7 +5,7 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js function ($, _, gettext, NotificationView, PromptView) { var toggleExpandCollapse, showLoadingIndicator, hideLoadingIndicator, confirmThenRunOperation, runOperationShowingMessage, disableElementWhileRunning, getScrollOffset, setScrollOffset, - setScrollTop, redirect, hasChangedAttributes; + setScrollTop, redirect, hasChangedAttributes, deleteNotificationHandler; /** * Toggles the expanded state of the current element. @@ -94,6 +94,21 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js }); }; + /** + * Returns a handler that removes a notification, both dismissing it and deleting it from the database. + * @param callback function to call when deletion succeeds + */ + deleteNotificationHandler = function(callback) { + return function (event) { + event.preventDefault(); + $.ajax({ + url: $(this).data('dismiss-link'), + type: 'DELETE', + success: callback + }); + }; + }; + /** * Performs an animated scroll so that the window has the specified scroll top. * @param scrollTop The desired scroll top for the window. @@ -158,6 +173,7 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js 'confirmThenRunOperation': confirmThenRunOperation, 'runOperationShowingMessage': runOperationShowingMessage, 'disableElementWhileRunning': disableElementWhileRunning, + 'deleteNotificationHandler': deleteNotificationHandler, 'setScrollTop': setScrollTop, 'getScrollOffset': getScrollOffset, 'setScrollOffset': setScrollOffset, From 7bf9c1a7fdd7f7e2b55adf66b6bcfde5110c4f11 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Tue, 19 Aug 2014 17:22:06 -0400 Subject: [PATCH 28/42] Add test for non URL characters --- cms/static/js/spec/views/pages/course_rerun_spec.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cms/static/js/spec/views/pages/course_rerun_spec.js b/cms/static/js/spec/views/pages/course_rerun_spec.js index 8bd31809c3..5d5204148d 100644 --- a/cms/static/js/spec/views/pages/course_rerun_spec.js +++ b/cms/static/js/spec/views/pages/course_rerun_spec.js @@ -1,5 +1,5 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers", "js/views/course_rerun", - "js/views/utils/create_course_utils"], + "js/views/utils/create_course_utils", "jquery.simulate"], function ($, create_sinon, view_helpers, CourseRerunUtils, CreateCourseUtilsFactory) { describe("Create course rerun page", function () { var selectors = { @@ -100,6 +100,14 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" setErrorMessage(selectors.org, ''); expect($(selectors.save)).toHaveClass(classes.disabled); }); + + it("shows an error message when non URL characters are entered", function () { + var input = $(selectors.org); + expect(input.parent()).not.toHaveClass(classes.error); + input.val("%") + input.simulate("keyup", { keyCode: $.simulate.keyCode.ENTER }); + expect(input.parent()).toHaveClass(classes.error); + }); }); it("saves course reruns", function () { From baee2da27835af5f2bc84efd39b5a307dc60f532 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Wed, 20 Aug 2014 11:30:35 -0400 Subject: [PATCH 29/42] Improve javascript test coverage --- cms/static/js/index.js | 2 +- .../js/spec/views/pages/course_rerun_spec.js | 63 +++++++++++++++++-- cms/static/js/spec/views/pages/index_spec.js | 37 +++++++++-- cms/static/js/views/course_rerun.js | 6 +- .../js/views/utils/create_course_utils.js | 6 +- cms/static/js/views/utils/view_utils.js | 10 ++- 6 files changed, 108 insertions(+), 16 deletions(-) diff --git a/cms/static/js/index.js b/cms/static/js/index.js index 26206859eb..70f036012f 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -82,7 +82,7 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie var onReady = function () { $('.new-course-button').bind('click', addNewCourse); $('.dismiss-button').bind('click', ViewUtils.deleteNotificationHandler(function () { - window.location.reload(); + ViewUtils.reload(); })); }; diff --git a/cms/static/js/spec/views/pages/course_rerun_spec.js b/cms/static/js/spec/views/pages/course_rerun_spec.js index 5d5204148d..5a9eb0b87b 100644 --- a/cms/static/js/spec/views/pages/course_rerun_spec.js +++ b/cms/static/js/spec/views/pages/course_rerun_spec.js @@ -1,6 +1,6 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers", "js/views/course_rerun", - "js/views/utils/create_course_utils", "jquery.simulate"], - function ($, create_sinon, view_helpers, CourseRerunUtils, CreateCourseUtilsFactory) { + "js/views/utils/create_course_utils", "js/views/utils/view_utils", "jquery.simulate"], + function ($, create_sinon, view_helpers, CourseRerunUtils, CreateCourseUtilsFactory, ViewUtils) { describe("Create course rerun page", function () { var selectors = { org: '.rerun-course-org', @@ -66,6 +66,11 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" return element; }; + var type = function (input, value) { + input.val(value); + input.simulate("keyup", { keyCode: $.simulate.keyCode.SPACE }); + }; + it("shows an error message", function () { var element = setErrorMessage(selectors.org, 'error message'); expect(element).toHaveClass(classes.error); @@ -104,15 +109,55 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" it("shows an error message when non URL characters are entered", function () { var input = $(selectors.org); expect(input.parent()).not.toHaveClass(classes.error); - input.val("%") + type(input, "%"); + expect(input.parent()).toHaveClass(classes.error); + }); + + it("does not show an error message when tabbing into a field", function () { + var input = $(selectors.number); + input.val(''); + expect(input.parent()).not.toHaveClass(classes.error); + input.simulate("keyup", { keyCode: $.simulate.keyCode.TAB }); + expect(input.parent()).not.toHaveClass(classes.error); + }); + + it("shows an error message when a required field is empty", function () { + var input = $(selectors.org); + input.val(''); + expect(input.parent()).not.toHaveClass(classes.error); input.simulate("keyup", { keyCode: $.simulate.keyCode.ENTER }); expect(input.parent()).toHaveClass(classes.error); }); + + it("shows an error message when spaces are entered and unicode is allowed", function () { + var input = $(selectors.org); + $(selectors.allowUnicode).val('True'); + expect(input.parent()).not.toHaveClass(classes.error); + type(input, ' '); + expect(input.parent()).toHaveClass(classes.error); + }); + + it("shows an error message when total length exceeds 65 characters", function () { + expect($(selectors.errorWrapper)).not.toHaveClass(classes.shown); + type($(selectors.org), 'ThisIsAVeryLongNameThatWillExceedTheSixtyFiveCharacterLimit'); + type($(selectors.number), 'ThisIsAVeryLongNameThatWillExceedTheSixtyFiveCharacterLimit'); + type($(selectors.run), 'ThisIsAVeryLongNameThatWillExceedTheSixtyFiveCharacterLimit'); + expect($(selectors.errorWrapper)).toHaveClass(classes.shown); + }); + + describe("Name field", function () { + it("does not show an error message when non URL characters are entered", function () { + var input = $(selectors.name); + expect(input.parent()).not.toHaveClass(classes.error); + type(input, "%"); + expect(input.parent()).not.toHaveClass(classes.error); + }); + }); }); it("saves course reruns", function () { var requests = create_sinon.requests(this); - window.source_course_key = 'test_course_key'; + var redirectSpy = spyOn(ViewUtils, 'redirect') fillInFields('DemoX', 'DM101', '2014', 'Demo course'); $(selectors.save).click(); create_sinon.expectJsonRequest(requests, 'POST', '/course/', { @@ -125,6 +170,10 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" expect($(selectors.save)).toHaveClass(classes.disabled); expect($(selectors.save)).toHaveClass(classes.processing); expect($(selectors.cancel)).toHaveClass(classes.hidden); + create_sinon.respondWithJson(requests, { + url: 'dummy_test_url' + }); + expect(redirectSpy).toHaveBeenCalledWith('dummy_test_url'); }); it("displays an error when saving fails", function () { @@ -146,5 +195,11 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" $(selectors.save).click(); expect(requests.length).toBe(0); }); + + it("can be canceled", function () { + var redirectSpy = spyOn(ViewUtils, 'redirect'); + $(selectors.cancel).click(); + expect(redirectSpy).toHaveBeenCalledWith('/course/'); + }); }); }); diff --git a/cms/static/js/spec/views/pages/index_spec.js b/cms/static/js/spec/views/pages/index_spec.js index 5eafbc476d..cadba58a22 100644 --- a/cms/static/js/spec/views/pages/index_spec.js +++ b/cms/static/js/spec/views/pages/index_spec.js @@ -1,7 +1,15 @@ -define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers", "js/index"], - function ($, create_sinon, view_helpers, IndexUtils) { +define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers", "js/index", + "js/views/utils/view_utils"], + function ($, create_sinon, view_helpers, IndexUtils, ViewUtils) { describe("Course listing page", function () { - var mockIndexPageHTML = readFixtures('mock/mock-index-page.underscore'); + var mockIndexPageHTML = readFixtures('mock/mock-index-page.underscore'), fillInFields; + + var fillInFields = function (org, number, run, name) { + $('.new-course-org').val(org); + $('.new-course-number').val(number); + $('.new-course-run').val(run); + $('.new-course-name').val(name); + }; beforeEach(function () { view_helpers.installMockAnalytics(); @@ -16,8 +24,29 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" it("can dismiss notifications", function () { var requests = create_sinon.requests(this); + var reloadSpy = spyOn(ViewUtils, 'reload'); $('.dismiss-button').click(); create_sinon.expectJsonRequest(requests, 'DELETE', 'dummy_dismiss_url'); + create_sinon.respondToDelete(requests); + expect(reloadSpy).toHaveBeenCalled(); + }); + + it("saves new courses", function () { + var requests = create_sinon.requests(this); + var redirectSpy = spyOn(ViewUtils, 'redirect'); + $('.new-course-button').click() + fillInFields('DemoX', 'DM101', '2014', 'Demo course'); + $('.new-course-save').click(); + create_sinon.expectJsonRequest(requests, 'POST', '/course/', { + org: 'DemoX', + number: 'DM101', + run: '2014', + display_name: 'Demo course' + }); + create_sinon.respondWithJson(requests, { + url: 'dummy_test_url' + }); + expect(redirectSpy).toHaveBeenCalledWith('dummy_test_url'); }); }); - }); \ No newline at end of file + }); diff --git a/cms/static/js/views/course_rerun.js b/cms/static/js/views/course_rerun.js index 60fd88fd7d..bfa6370d1b 100644 --- a/cms/static/js/views/course_rerun.js +++ b/cms/static/js/views/course_rerun.js @@ -1,5 +1,5 @@ -define(["domReady", "jquery", "underscore", "js/views/utils/create_course_utils"], - function (domReady, $, _, CreateCourseUtilsFactory) { +define(["domReady", "jquery", "underscore", "js/views/utils/create_course_utils", "js/views/utils/view_utils"], + function (domReady, $, _, CreateCourseUtilsFactory, ViewUtils) { var CreateCourseUtils = CreateCourseUtilsFactory({ name: '.rerun-course-name', org: '.rerun-course-org', @@ -62,7 +62,7 @@ define(["domReady", "jquery", "underscore", "js/views/utils/create_course_utils" $('#course_rerun_error').html(''); $('wrapper-error').removeClass('is-shown').addClass('is-hidden'); $('.rerun-course-save').off('click'); - window.location.href = '/course/'; + ViewUtils.redirect('/course/'); }; var onReady = function () { diff --git a/cms/static/js/views/utils/create_course_utils.js b/cms/static/js/views/utils/create_course_utils.js index a8959fa247..c712e649d2 100644 --- a/cms/static/js/views/utils/create_course_utils.js +++ b/cms/static/js/views/utils/create_course_utils.js @@ -1,8 +1,8 @@ /** * Provides utilities for validating courses during creation, for both new courses and reruns. */ -define(["jquery", "underscore", "gettext"], - function ($, _, gettext) { +define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], + function ($, _, gettext, ViewUtils) { return function (selectors, classes) { var validateRequiredField, validateCourseItemEncoding, validateTotalCourseItemsLength, setNewCourseFieldInErr, hasInvalidRequiredFields, createCourse, validateFilledFields, configureHandlers; @@ -84,7 +84,7 @@ define(["jquery", "underscore", "gettext"], courseInfo, function (data) { if (data.url !== undefined) { - window.location = data.url; + ViewUtils.redirect(data.url); } else if (data.ErrMsg !== undefined) { errorHandler(data.ErrMsg); } diff --git a/cms/static/js/views/utils/view_utils.js b/cms/static/js/views/utils/view_utils.js index 9719ea9247..27d969f523 100644 --- a/cms/static/js/views/utils/view_utils.js +++ b/cms/static/js/views/utils/view_utils.js @@ -5,7 +5,7 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js function ($, _, gettext, NotificationView, PromptView) { var toggleExpandCollapse, showLoadingIndicator, hideLoadingIndicator, confirmThenRunOperation, runOperationShowingMessage, disableElementWhileRunning, getScrollOffset, setScrollOffset, - setScrollTop, redirect, hasChangedAttributes, deleteNotificationHandler; + setScrollTop, redirect, reload, hasChangedAttributes, deleteNotificationHandler; /** * Toggles the expanded state of the current element. @@ -147,6 +147,13 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js window.location = url; }; + /** + * Reloads the page. This is broken out as its own function for unit testing. + */ + reload = function() { + window.location.reload(); + }; + /** * Returns true if a model has changes to at least one of the specified attributes. * @param model The model in question. @@ -178,6 +185,7 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js 'getScrollOffset': getScrollOffset, 'setScrollOffset': setScrollOffset, 'redirect': redirect, + 'reload': reload, 'hasChangedAttributes': hasChangedAttributes }; }); From d9432041994f34065d734a2efef99e5b2c3dd690 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Wed, 20 Aug 2014 11:57:59 -0400 Subject: [PATCH 30/42] Fix i18n on course outline and show error message for long names --- cms/static/js/views/utils/create_course_utils.js | 4 ++-- cms/templates/course_outline.html | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cms/static/js/views/utils/create_course_utils.js b/cms/static/js/views/utils/create_course_utils.js index c712e649d2..2c0c3493ac 100644 --- a/cms/static/js/views/utils/create_course_utils.js +++ b/cms/static/js/views/utils/create_course_utils.js @@ -39,12 +39,12 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], }, 0 ); if (totalLength > 65) { - $(selectors.errorWrapper).addClass(classes.shown); + $(selectors.errorWrapper).addClass(classes.shown).removeClass(classes.hiding); $(selectors.errorMessage).html('

        ' + gettext('The combined length of the organization, course number, and course run fields cannot be more than 65 characters.') + '

        '); $(selectors.save).addClass(classes.disabled); } else { - $(selectors.errorWrapper).removeClass(classes.shown); + $(selectors.errorWrapper).removeClass(classes.shown).addClass(classes.hiding); } }; diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index 2fd4867b64..3e7629e770 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -43,9 +43,9 @@ from contentstore.utils import reverse_usage_url
        -

        This course was created as a re-run. Some manual configuration is needed.

        +

        ${_("This course was created as a re-run. Some manual configuration is needed.")}

        -

        Be sure to review and reset all dates (the Course Start Date was set to January 1, 2030); set up the course team; review course updates and other assets for dated material; and seed the discussions and wiki.

        +

        ${_("Be sure to review and reset all dates (the Course Start Date was set to January 1, 2030); set up the course team; review course updates and other assets for dated material; and seed the discussions and wiki.")}

          From 4e41a42c765384cb282f698f849af182ec87ba47 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Wed, 20 Aug 2014 12:05:54 -0400 Subject: [PATCH 31/42] Rename unsucceeded to in_process --- cms/djangoapps/contentstore/views/course.py | 26 ++++++++++----------- cms/templates/index.html | 4 ++-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 760db51924..40ea4aa564 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -285,14 +285,14 @@ def _accessible_courses_list(request): return has_course_access(request.user, course.id) courses = filter(course_filter, modulestore().get_courses()) - unsucceeded_course_actions = [ + in_process_course_actions = [ course for course in CourseRerunState.objects.find_all( exclude_args={'state': CourseRerunUIStateManager.State.SUCCEEDED}, should_display=True ) if has_course_access(request.user, course.course_key) ] - return courses, unsucceeded_course_actions + return courses, in_process_course_actions def _accessible_courses_list_from_groups(request): @@ -300,7 +300,7 @@ def _accessible_courses_list_from_groups(request): List all courses available to the logged in user by reversing access group names """ courses_list = {} - unsucceeded_course_actions = [] + in_process_course_actions = [] instructor_courses = UserBasedRole(request.user, CourseInstructorRole.ROLE).courses_with_role() staff_courses = UserBasedRole(request.user, CourseStaffRole.ROLE).courses_with_role() @@ -313,7 +313,7 @@ def _accessible_courses_list_from_groups(request): raise AccessListFallback if course_key not in courses_list: # check for any course action state for this course - unsucceeded_course_actions.extend( + in_process_course_actions.extend( CourseRerunState.objects.find_all( exclude_args={'state': CourseRerunUIStateManager.State.SUCCEEDED}, should_display=True, @@ -330,7 +330,7 @@ def _accessible_courses_list_from_groups(request): # ignore deleted or errored courses courses_list[course_key] = course - return courses_list.values(), unsucceeded_course_actions + return courses_list.values(), in_process_course_actions @login_required @@ -343,14 +343,14 @@ def course_listing(request): """ if GlobalStaff().has_user(request.user): # user has global access so no need to get courses from django groups - courses, unsucceeded_course_actions = _accessible_courses_list(request) + courses, in_process_course_actions = _accessible_courses_list(request) else: try: - courses, unsucceeded_course_actions = _accessible_courses_list_from_groups(request) + courses, in_process_course_actions = _accessible_courses_list_from_groups(request) except AccessListFallback: # user have some old groups or there was some error getting courses from django groups # so fallback to iterating through all courses - courses, unsucceeded_course_actions = _accessible_courses_list(request) + courses, in_process_course_actions = _accessible_courses_list(request) def format_course_for_view(course): """ @@ -385,19 +385,19 @@ def course_listing(request): }) if uca.state == CourseRerunUIStateManager.State.FAILED else '' } - # remove any courses in courses that are also in the unsucceeded_course_actions list - unsucceeded_action_course_keys = [uca.course_key for uca in unsucceeded_course_actions] + # remove any courses in courses that are also in the in_process_course_actions list + in_process_action_course_keys = [uca.course_key for uca in in_process_course_actions] courses = [ format_course_for_view(c) for c in courses - if not isinstance(c, ErrorDescriptor) and (c.id not in unsucceeded_action_course_keys) + if not isinstance(c, ErrorDescriptor) and (c.id not in in_process_action_course_keys) ] - unsucceeded_course_actions = [format_unsucceeded_course_for_view(uca) for uca in unsucceeded_course_actions] + in_process_course_actions = [format_unsucceeded_course_for_view(uca) for uca in in_process_course_actions] return render_to_response('index.html', { 'courses': courses, - 'unsucceeded_course_actions': unsucceeded_course_actions, + 'in_process_course_actions': in_process_course_actions, 'user': request.user, 'request_course_creator_url': reverse('contentstore.views.request_course_creator'), 'course_creator_status': _get_course_creator_status(request.user), diff --git a/cms/templates/index.html b/cms/templates/index.html index 72b384f4bf..2e546d564a 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -132,12 +132,12 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) { % endif - %if allow_course_reruns and rerun_creator_status and len(unsucceeded_course_actions) > 0: + %if allow_course_reruns and rerun_creator_status and len(in_process_course_actions) > 0:

          Courses Being Processed

            - %for course_info in sorted(unsucceeded_course_actions, key=lambda s: s['display_name'].lower() if s['display_name'] is not None else ''): + %for course_info in sorted(in_process_course_actions, key=lambda s: s['display_name'].lower() if s['display_name'] is not None else ''): %if course_info['is_in_progress']:
          • From ab3a8c96d32729f8105d9590eec31b3acf0e0799 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Wed, 20 Aug 2014 12:54:47 -0400 Subject: [PATCH 32/42] Add test for failed course creation --- cms/static/js/spec/views/pages/index_spec.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/cms/static/js/spec/views/pages/index_spec.js b/cms/static/js/spec/views/pages/index_spec.js index cadba58a22..fab5b9653e 100644 --- a/cms/static/js/spec/views/pages/index_spec.js +++ b/cms/static/js/spec/views/pages/index_spec.js @@ -48,5 +48,18 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" }); expect(redirectSpy).toHaveBeenCalledWith('dummy_test_url'); }); + + it("displays an error when saving fails", function () { + var requests = create_sinon.requests(this); + $('.new-course-button').click(); + fillInFields('DemoX', 'DM101', '2014', 'Demo course'); + $('.new-course-save').click(); + create_sinon.respondWithJson(requests, { + ErrMsg: 'error message' + }); + expect($('.wrap-error')).toHaveClass('is-shown'); + expect($('#course_creation_error')).toContainText('error message'); + expect($('.new-course-save')).toHaveClass('is-disabled'); + }); }); }); From de52f8235222d1f07291cfb4d2ac8e62bc218ff4 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Wed, 20 Aug 2014 13:24:24 -0400 Subject: [PATCH 33/42] PEP8/pylint cleanup --- cms/djangoapps/contentstore/views/course.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 40ea4aa564..682cbc1991 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -231,6 +231,7 @@ def course_handler(request, course_key_string=None): else: return HttpResponseNotFound() + @login_required @ensure_csrf_cookie @require_http_methods(["GET"]) @@ -367,7 +368,7 @@ def course_listing(request): 'run': course.location.run } - def format_unsucceeded_course_for_view(uca): + def format_in_process_course_view(uca): """ Return a dict of the data which the view requires for each unsucceeded course """ @@ -393,7 +394,7 @@ def course_listing(request): if not isinstance(c, ErrorDescriptor) and (c.id not in in_process_action_course_keys) ] - in_process_course_actions = [format_unsucceeded_course_for_view(uca) for uca in in_process_course_actions] + in_process_course_actions = [format_in_process_course_view(uca) for uca in in_process_course_actions] return render_to_response('index.html', { 'courses': courses, @@ -408,6 +409,7 @@ def course_listing(request): def _get_rerun_link_for_item(course_key): + """ Returns the rerun link for the given course key. """ return reverse_course_url('course_rerun_handler', course_key) @@ -446,7 +448,8 @@ def course_index(request, course_key): 'rerun_notification_id': current_action.id if current_action else None, 'course_release_date': course_release_date, 'settings_url': settings_url, - 'notification_dismiss_url': reverse_course_url('course_notifications_handler', current_action.course_key, kwargs={ + 'notification_dismiss_url': + reverse_course_url('course_notifications_handler', current_action.course_key, kwargs={ 'action_state_id': current_action.id, }) if current_action else None, }) From 4bc672b93db1392170367ceff08b06993b69d766 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Wed, 20 Aug 2014 14:54:21 -0400 Subject: [PATCH 34/42] Fix broken unicode char in CSS --- cms/static/sass/_base.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/static/sass/_base.scss b/cms/static/sass/_base.scss index 7885243985..547d9b2e39 100644 --- a/cms/static/sass/_base.scss +++ b/cms/static/sass/_base.scss @@ -789,7 +789,7 @@ hr.divide { &:after { @include font-size(20); - content: '???'; + content: '▾'; display: block; position: absolute; bottom: -14px; From 85f5c49bb79941c0a9f743449c379a9a9cc3a944 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 20 Aug 2014 14:56:49 -0400 Subject: [PATCH 35/42] Unit tests for rerun along w/ fixes to make them work LMS-11011 --- .../contentstore/tests/test_clone_course.py | 46 +++++++++++++++++++ cms/djangoapps/contentstore/views/course.py | 2 +- .../lib/xmodule/xmodule/modulestore/mixed.py | 6 +-- .../xmodule/modulestore/split_mongo/split.py | 6 ++- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_clone_course.py b/cms/djangoapps/contentstore/tests/test_clone_course.py index acd2d93714..5f91596d68 100644 --- a/cms/djangoapps/contentstore/tests/test_clone_course.py +++ b/cms/djangoapps/contentstore/tests/test_clone_course.py @@ -4,6 +4,11 @@ Unit tests for cloning a course between the same and different module stores. from opaque_keys.edx.locator import CourseLocator from xmodule.modulestore import ModuleStoreEnum from contentstore.tests.utils import CourseTestCase +from contentstore.tasks import rerun_course +from contentstore.views.access import has_course_access +from course_action_state.models import CourseRerunState +from course_action_state.managers import CourseRerunUIStateManager +from mock import patch, Mock class CloneCourseTest(CourseTestCase): @@ -39,3 +44,44 @@ class CloneCourseTest(CourseTestCase): ) self.store.clone_course(split_course3_id, split_course4_id, self.user.id) self.assertCoursesEqual(split_course3_id, split_course4_id) + + def test_rerun_course(self): + """ + Unit tests for :meth: `contentstore.tasks.rerun_course` + """ + mongo_course1_id = self.import_and_populate_course() + + # rerun from mongo into split + split_course3_id = CourseLocator( + org="edx3", course="split3", run="rerun_test" + ) + # Mark the action as initiated + fields = {'display_name': 'rerun'} + CourseRerunState.objects.initiated(mongo_course1_id, split_course3_id, self.user, fields['display_name']) + result = rerun_course.delay(unicode(mongo_course1_id), unicode(split_course3_id), self.user.id, fields) + self.assertEqual(result.get(), "succeeded") + self.assertTrue(has_course_access(self.user, split_course3_id), "Didn't grant access") + rerun_state = CourseRerunState.objects.find_first(course_key=split_course3_id) + self.assertEqual(rerun_state.state, CourseRerunUIStateManager.State.SUCCEEDED) + + # try creating rerunning again to same name and ensure it generates error + result = rerun_course.delay(unicode(mongo_course1_id), unicode(split_course3_id), self.user.id) + self.assertEqual(result.get(), "duplicate course") + # the below will raise an exception if the record doesn't exist + CourseRerunState.objects.find_first( + course_key=split_course3_id, + state=CourseRerunUIStateManager.State.FAILED + ) + + # try to hit the generic exception catch + with patch('xmodule.modulestore.split_mongo.mongo_connection.MongoConnection.insert_course_index', Mock(side_effect=Exception)): + split_course4_id = CourseLocator(org="edx3", course="split3", run="rerun_fail") + fields = {'display_name': 'total failure'} + CourseRerunState.objects.initiated(split_course3_id, split_course4_id, self.user, fields['display_name']) + result = rerun_course.delay(unicode(split_course3_id), unicode(split_course4_id), self.user.id, fields) + self.assertIn("exception: ", result.get()) + self.assertIsNone(self.store.get_course(split_course4_id), "Didn't delete course after error") + CourseRerunState.objects.find_first( + course_key=split_course4_id, + state=CourseRerunUIStateManager.State.FAILED + ) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 682cbc1991..e2e88c636e 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -599,7 +599,7 @@ def _rerun_course(request, org, number, run, fields): with store.default_store('split'): destination_course_key = store.make_course_key(org, number, run) - # verify org course and run don't already exist + # verify org course and run don't already exist if store.has_course(destination_course_key, ignore_case=True): raise DuplicateCourseError(source_course_key, destination_course_key) diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 26fbf769e7..5cb862c50b 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -393,15 +393,13 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): if source_modulestore == dest_modulestore: return source_modulestore.clone_course(source_course_id, dest_course_id, user_id, fields, **kwargs) - # ensure super's only called once. The delegation above probably calls it; so, don't move - # the invocation above the delegation call - super(MixedModuleStore, self).clone_course(source_course_id, dest_course_id, user_id, fields, **kwargs) - if dest_modulestore.get_modulestore_type() == ModuleStoreEnum.Type.split: split_migrator = SplitMigrator(dest_modulestore, source_modulestore) split_migrator.migrate_mongo_course( source_course_id, user_id, dest_course_id.org, dest_course_id.course, dest_course_id.run, fields, **kwargs ) + # the super handles assets and any other necessities + super(MixedModuleStore, self).clone_course(source_course_id, dest_course_id, user_id, fields, **kwargs) @strip_key def create_item(self, user_id, course_key, block_type, block_id=None, fields=None, **kwargs): diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index f576ea67e0..05084d85f3 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -952,11 +952,10 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): In split, other than copying the assets, this is cheap as it merely creates a new version of the existing course. """ - super(SplitMongoModuleStore, self).clone_course(source_course_id, dest_course_id, user_id, fields, **kwargs) source_index = self.get_course_index_info(source_course_id) if source_index is None: raise ItemNotFoundError("Cannot find a course at {0}. Aborting".format(source_course_id)) - return self.create_course( + new_course = self.create_course( dest_course_id.org, dest_course_id.course, dest_course_id.run, user_id, fields=fields, @@ -965,6 +964,9 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): skip_auto_publish=True, **kwargs ) + # don't copy assets until we create the course in case something's awry + super(SplitMongoModuleStore, self).clone_course(source_course_id, dest_course_id, user_id, fields, **kwargs) + return new_course DEFAULT_ROOT_BLOCK_ID = 'course' def create_course( From 3b6b4668dd696e483623e28408c7c5555a222292 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 20 Aug 2014 15:57:55 -0400 Subject: [PATCH 36/42] On rerun, override the course start date LMS-11011 --- .../tests/test_course_create_rerun.py | 67 +++++++++++++++++++ cms/djangoapps/contentstore/views/course.py | 15 +++-- 2 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 cms/djangoapps/contentstore/tests/test_course_create_rerun.py diff --git a/cms/djangoapps/contentstore/tests/test_course_create_rerun.py b/cms/djangoapps/contentstore/tests/test_course_create_rerun.py new file mode 100644 index 0000000000..e23f5f4236 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_course_create_rerun.py @@ -0,0 +1,67 @@ +""" +Test view handler for rerun (and eventually create) +""" +from django.test.client import RequestFactory +from opaque_keys.edx.keys import CourseKey + +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +from student.roles import CourseInstructorRole, CourseStaffRole +from student.tests.factories import UserFactory +from contentstore.tests.utils import AjaxEnabledTestClient, parse_json +from datetime import datetime +from xmodule.course_module import CourseFields + + +class TestCourseListing(ModuleStoreTestCase): + """ + Unit tests for getting the list of courses for a logged in user + """ + def setUp(self): + """ + Add a user and a course + """ + super(TestCourseListing, self).setUp() + # create and log in a staff user. + # create and log in a non-staff user + self.user = UserFactory() + self.factory = RequestFactory() + self.client = AjaxEnabledTestClient() + self.client.login(username=self.user.username, password='test') + + source_course = CourseFactory.create( + org='origin', + number='the_beginning', + run='first', + display_name='the one and only', + start=datetime.utcnow() + ) + self.source_course_key = source_course.id + + for role in [CourseInstructorRole, CourseStaffRole]: + role(self.source_course_key).add_users(self.user) + + + def tearDown(self): + """ + Reverse the setup + """ + self.client.logout() + ModuleStoreTestCase.tearDown(self) + + def test_rerun(self): + """ + Just testing the functionality the view handler adds over the tasks tested in test_clone_course + """ + response = self.client.ajax_post('/course/', { + 'source_course_key': unicode(self.source_course_key), + 'org': self.source_course_key.org, 'course': self.source_course_key.course, 'run': 'copy', + 'display_name': 'not the same old name', + }) + self.assertEqual(response.status_code, 200) + data = parse_json(response) + dest_course_key = CourseKey.from_string(data['destination_course_key']) + + self.assertEqual(dest_course_key.run, 'copy') + dest_course = self.store.get_course(dest_course_key) + self.assertEqual(dest_course.start, CourseFields.start.default) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index e2e88c636e..c6d97ec54d 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -67,6 +67,7 @@ from student import auth from course_action_state.models import CourseRerunState, CourseRerunUIStateManager from course_action_state.managers import CourseActionStateItemNotFoundError from microsite_configuration import microsite +from xmodule.course_module import CourseFields __all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler', @@ -508,24 +509,28 @@ def _create_or_rerun_course(request): try: org = request.json.get('org') - number = request.json.get('number') + course = request.json.get('number', request.json.get('course')) display_name = request.json.get('display_name') + # force the start date for reruns and allow us to override start via the client + start = request.json.get('start', CourseFields.start.default) run = request.json.get('run') # allow/disable unicode characters in course_id according to settings if not settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID'): - if _has_non_ascii_characters(org) or _has_non_ascii_characters(number) or _has_non_ascii_characters(run): + if _has_non_ascii_characters(org) or _has_non_ascii_characters(course) or _has_non_ascii_characters(run): return JsonResponse( {'error': _('Special characters not allowed in organization, course number, and course run.')}, status=400 ) - fields = {'display_name': display_name} if display_name is not None else {} + fields = {'start': start} + if display_name is not None: + fields['display_name'] = display_name if 'source_course_key' in request.json: - return _rerun_course(request, org, number, run, fields) + return _rerun_course(request, org, course, run, fields) else: - return _create_new_course(request, org, number, run, fields) + return _create_new_course(request, org, course, run, fields) except DuplicateCourseError: return JsonResponse({ From 883aedb51b415df194212cbc482c4890fcade66c Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Wed, 20 Aug 2014 16:24:52 -0400 Subject: [PATCH 37/42] Add blank=True to rerun state display name --- .../migrations/0002_add_rerun_display_name.py | 2 +- common/djangoapps/course_action_state/models.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py b/common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py index fc02006aff..92b2cc24a0 100644 --- a/common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py +++ b/common/djangoapps/course_action_state/migrations/0002_add_rerun_display_name.py @@ -10,7 +10,7 @@ class Migration(SchemaMigration): def forwards(self, orm): # Adding field 'CourseRerunState.display_name' db.add_column('course_action_state_coursererunstate', 'display_name', - self.gf('django.db.models.fields.CharField')(default='', max_length=255), + self.gf('django.db.models.fields.CharField')(default='', max_length=255, blank=True), keep_default=False) diff --git a/common/djangoapps/course_action_state/models.py b/common/djangoapps/course_action_state/models.py index a6a4c23de2..48ca83f47f 100644 --- a/common/djangoapps/course_action_state/models.py +++ b/common/djangoapps/course_action_state/models.py @@ -110,7 +110,7 @@ class CourseRerunState(CourseActionUIState): source_course_key = CourseKeyField(max_length=255, db_index=True) # Display name for destination course - display_name = models.CharField(max_length=255, default="") + display_name = models.CharField(max_length=255, default="", blank=True) # MANAGERS # Override the abstract class' manager with a Rerun-specific manager that inherits from the base class' manager. From b6d9ef3ed7e146fe43c04e5f63857037cf5b33ab Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Wed, 20 Aug 2014 17:05:27 -0400 Subject: [PATCH 38/42] More PEP8/pylint cleanup --- .../contentstore/tests/test_course_create_rerun.py | 1 - cms/djangoapps/contentstore/views/course.py | 2 -- cms/envs/devstack.py | 7 ------- 3 files changed, 10 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_course_create_rerun.py b/cms/djangoapps/contentstore/tests/test_course_create_rerun.py index e23f5f4236..a7c5df8bc7 100644 --- a/cms/djangoapps/contentstore/tests/test_course_create_rerun.py +++ b/cms/djangoapps/contentstore/tests/test_course_create_rerun.py @@ -41,7 +41,6 @@ class TestCourseListing(ModuleStoreTestCase): for role in [CourseInstructorRole, CourseStaffRole]: role(self.source_course_key).add_users(self.user) - def tearDown(self): """ Reverse the setup diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index c6d97ec54d..d3cafad5ca 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -29,8 +29,6 @@ from opaque_keys.edx.locations import Location from opaque_keys.edx.keys import CourseKey from django_future.csrf import ensure_csrf_cookie -from util.json_request import JsonResponse -from edxmako.shortcuts import render_to_response from contentstore.course_info_model import get_course_updates, update_course_updates, delete_course_update from contentstore.utils import ( add_instructor, diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 67bf940d40..05986e8b3d 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -38,15 +38,8 @@ FEATURES['ALLOW_ALL_ADVANCED_COMPONENTS'] = True ################################# CELERY ###################################### # By default don't use a worker, execute tasks as if they were local functions - -# TODO BEWARE: UNCOMMENT THIS BEFORE MERGING INTO MASTER CELERY_ALWAYS_EAGER = True -# TODO BEWARE: DO NOT COMMIT THE REST OF THIS SECTION INTO MASTER - FOR LOCAL TESTING ONLY -# Test with Celery threads -# CELERY_ALWAYS_EAGER = False -# BROKER_URL = 'redis://' - ################################ COURSE RERUNS ################################ FEATURES['ALLOW_COURSE_RERUNS'] = True From bb90c3076733199b8ab6d00182bec8e69aedaa53 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Wed, 20 Aug 2014 17:34:46 -0400 Subject: [PATCH 39/42] Make refresh clickable while courses are processing --- cms/static/js/index.js | 1 + cms/templates/index.html | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cms/static/js/index.js b/cms/static/js/index.js index 70f036012f..c984310197 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -84,6 +84,7 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie $('.dismiss-button').bind('click', ViewUtils.deleteNotificationHandler(function () { ViewUtils.reload(); })); + $('.action-reload').bind('click', ViewUtils.reload); }; domReady(onReady); diff --git a/cms/templates/index.html b/cms/templates/index.html index 2e546d564a..eed1669c58 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -169,7 +169,7 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {
          -

          ${_("The new course will be added to your course list in 5-10 minutes. Return to this page or refresh it to update the course list. The new course will need some manual configuration.")}

          +

          ${_('The new course will be added to your course list in 5-10 minutes. Return to this page or refresh it to update the course list. The new course will need some manual configuration.')}

          %endif From ff525bea96c0e9d8814437ac8881995e8911fab4 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Thu, 21 Aug 2014 11:16:50 -0400 Subject: [PATCH 40/42] Fix field serialization --- cms/djangoapps/contentstore/tasks.py | 12 +++++++++++- cms/djangoapps/contentstore/views/course.py | 4 +++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 8e309f2bad..ac1bfa9c48 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -4,7 +4,9 @@ This file contains celery tasks for contentstore views from celery.task import task from django.contrib.auth.models import User +import json from xmodule.modulestore.django import modulestore +from xmodule.course_module import CourseFields from xmodule.modulestore.exceptions import DuplicateCourseError, ItemNotFoundError from course_action_state.models import CourseRerunState @@ -18,9 +20,10 @@ def rerun_course(source_course_key_string, destination_course_key_string, user_i Reruns a course in a new celery task. """ try: - # deserialize the keys + # deserialize the payload source_course_key = CourseKey.from_string(source_course_key_string) destination_course_key = CourseKey.from_string(destination_course_key_string) + fields = deserialize_fields(fields) # use the split modulestore as the store for the rerun course, # as the Mongo modulestore doesn't support multiple runs of the same course. @@ -53,3 +56,10 @@ def rerun_course(source_course_key_string, destination_course_key_string, user_i pass return "exception: " + unicode(exc) + + +def deserialize_fields(json_fields): + fields = json.loads(json_fields) + for field_name, value in fields.iteritems(): + fields[field_name] = getattr(CourseFields, field_name).from_json(value) + return fields diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index d3cafad5ca..432bd250a5 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -23,6 +23,7 @@ from xmodule.modulestore.django import modulestore from xmodule.contentstore.content import StaticContent from xmodule.tabs import PDFTextbookTabs from xmodule.partitions.partitions import UserPartition, Group +from xmodule.modulestore import EdxJSONEncoder from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError from opaque_keys import InvalidKeyError from opaque_keys.edx.locations import Location @@ -614,7 +615,8 @@ def _rerun_course(request, org, number, run, fields): CourseRerunState.objects.initiated(source_course_key, destination_course_key, request.user, fields['display_name']) # Rerun the course as a new celery task - rerun_course.delay(unicode(source_course_key), unicode(destination_course_key), request.user.id, fields) + json_fields = json.dumps(fields, cls=EdxJSONEncoder) + rerun_course.delay(unicode(source_course_key), unicode(destination_course_key), request.user.id, json_fields) # Return course listing page return JsonResponse({ From d38bd2ea958eca30944373e1f0f2ed1b7d5c852b Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Thu, 21 Aug 2014 14:11:43 -0400 Subject: [PATCH 41/42] Reword rerun notification --- cms/templates/course_outline.html | 2 +- cms/templates/js/mock/mock-course-rerun-notification.underscore | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index 3e7629e770..193d239d37 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -45,7 +45,7 @@ from contentstore.utils import reverse_usage_url

          ${_("This course was created as a re-run. Some manual configuration is needed.")}

          -

          ${_("Be sure to review and reset all dates (the Course Start Date was set to January 1, 2030); set up the course team; review course updates and other assets for dated material; and seed the discussions and wiki.")}

          +

          ${_("Be sure to review and reset all dates (the Course Start Date has been changed to a future date); set up the course team; review course updates and other assets for dated material; and seed the discussions and wiki.")}

            diff --git a/cms/templates/js/mock/mock-course-rerun-notification.underscore b/cms/templates/js/mock/mock-course-rerun-notification.underscore index 68a0ddea8b..1081d7fc19 100644 --- a/cms/templates/js/mock/mock-course-rerun-notification.underscore +++ b/cms/templates/js/mock/mock-course-rerun-notification.underscore @@ -6,7 +6,7 @@

            This course was created as a re-run. Some manual configuration is needed.

            -

            Be sure to review and reset all dates (the Course Start Date was set to January 1, 2030); set up the +

            Be sure to review and reset all dates (the Course Start Date has been changed to a future date); set up the course team; review course updates and other assets for dated material; and seed the discussions and wiki.

            From 62643a5941889f64f067afe054f27e50d45795d0 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Thu, 21 Aug 2014 15:43:22 -0400 Subject: [PATCH 42/42] Fix broken rerun test --- cms/djangoapps/contentstore/tasks.py | 2 +- cms/djangoapps/contentstore/tests/test_clone_course.py | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index ac1bfa9c48..8b05565eb8 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -23,7 +23,7 @@ def rerun_course(source_course_key_string, destination_course_key_string, user_i # deserialize the payload source_course_key = CourseKey.from_string(source_course_key_string) destination_course_key = CourseKey.from_string(destination_course_key_string) - fields = deserialize_fields(fields) + fields = deserialize_fields(fields) if fields else None # use the split modulestore as the store for the rerun course, # as the Mongo modulestore doesn't support multiple runs of the same course. diff --git a/cms/djangoapps/contentstore/tests/test_clone_course.py b/cms/djangoapps/contentstore/tests/test_clone_course.py index 5f91596d68..25ea5eb6e8 100644 --- a/cms/djangoapps/contentstore/tests/test_clone_course.py +++ b/cms/djangoapps/contentstore/tests/test_clone_course.py @@ -1,8 +1,9 @@ """ Unit tests for cloning a course between the same and different module stores. """ +import json from opaque_keys.edx.locator import CourseLocator -from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore import ModuleStoreEnum, EdxJSONEncoder from contentstore.tests.utils import CourseTestCase from contentstore.tasks import rerun_course from contentstore.views.access import has_course_access @@ -58,7 +59,8 @@ class CloneCourseTest(CourseTestCase): # Mark the action as initiated fields = {'display_name': 'rerun'} CourseRerunState.objects.initiated(mongo_course1_id, split_course3_id, self.user, fields['display_name']) - result = rerun_course.delay(unicode(mongo_course1_id), unicode(split_course3_id), self.user.id, fields) + result = rerun_course.delay(unicode(mongo_course1_id), unicode(split_course3_id), self.user.id, + json.dumps(fields, cls=EdxJSONEncoder)) self.assertEqual(result.get(), "succeeded") self.assertTrue(has_course_access(self.user, split_course3_id), "Didn't grant access") rerun_state = CourseRerunState.objects.find_first(course_key=split_course3_id) @@ -78,7 +80,8 @@ class CloneCourseTest(CourseTestCase): split_course4_id = CourseLocator(org="edx3", course="split3", run="rerun_fail") fields = {'display_name': 'total failure'} CourseRerunState.objects.initiated(split_course3_id, split_course4_id, self.user, fields['display_name']) - result = rerun_course.delay(unicode(split_course3_id), unicode(split_course4_id), self.user.id, fields) + result = rerun_course.delay(unicode(split_course3_id), unicode(split_course4_id), self.user.id, + json.dumps(fields, cls=EdxJSONEncoder)) self.assertIn("exception: ", result.get()) self.assertIsNone(self.store.get_course(split_course4_id), "Didn't delete course after error") CourseRerunState.objects.find_first(