Disable setting course pacing during course run.

Also adds improved styling for course pacing settings, and unit tests
around query counts for self-paced courses.

ECOM-2650

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -116,11 +116,21 @@ class CourseDetailsTestCase(CourseTestCase):
             CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).effort,
             jsondetails.effort, "After set effort"
         )
+        jsondetails.self_paced = True
+        self.assertEqual(
+            CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).self_paced,
+            jsondetails.self_paced
+        )
         jsondetails.start_date = datetime.datetime(2010, 10, 1, 0, tzinfo=UTC())
         self.assertEqual(
             CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).start_date,
             jsondetails.start_date
         )
+        jsondetails.end_date = datetime.datetime(2011, 10, 1, 0, tzinfo=UTC())
+        self.assertEqual(
+            CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).end_date,
+            jsondetails.end_date
+        )
         jsondetails.course_image_name = "an_image.jpg"
         self.assertEqual(
             CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).course_image_name,
@@ -131,11 +141,6 @@ class CourseDetailsTestCase(CourseTestCase):
             CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).language,
             jsondetails.language
         )
-        jsondetails.self_paced = True
-        self.assertEqual(
-            CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).self_paced,
-            jsondetails.self_paced
-        )
 
     @override_settings(MKTG_URLS={'ROOT': 'dummy-root'})
     def test_marketing_site_fetch(self):
@@ -291,6 +296,19 @@ class CourseDetailsTestCase(CourseTestCase):
             self.assertContains(response, "Course Introduction Video")
             self.assertContains(response, "Requirements")
 
+    def test_toggle_pacing_during_course_run(self):
+        SelfPacedConfiguration(enabled=True).save()
+        self.course.start = datetime.datetime.now()
+        modulestore().update_item(self.course, self.user.id)
+
+        details = CourseDetails.fetch(self.course.id)
+        updated_details = CourseDetails.update_from_json(
+            self.course.id,
+            dict(details.__dict__, self_paced=True),
+            self.user
+        )
+        self.assertFalse(updated_details.self_paced)
+
 
 @ddt.ddt
 class CourseDetailsViewTest(CourseTestCase):

cms/djangoapps/models/settings/course_details.py

@@ -192,6 +192,7 @@ class CourseDetails(object):
             dirty = True
 
         if (SelfPacedConfiguration.current().enabled
+                and descriptor.can_toggle_course_pacing
                 and 'self_paced' in jsondict
                 and jsondict['self_paced'] != descriptor.self_paced):
             descriptor.self_paced = jsondict['self_paced']

cms/static/js/models/settings/course_details.js

@@ -70,6 +70,7 @@ var CourseDetails = Backbone.Model.extend({
     },
 
     _videokey_illegal_chars : /[^a-zA-Z0-9_-]/g,
+
     set_videosource: function(newsource) {
         // newsource either is <video youtube="speed:key, *"/> or just the "speed:key, *" string
         // returns the videosource for the preview which iss the key whose speed is closest to 1
@@ -81,9 +82,16 @@ var CourseDetails = Backbone.Model.extend({
 
         return this.videosourceSample();
     },
+
     videosourceSample : function() {
         if (this.has('intro_video')) return "//www.youtube.com/embed/" + this.get('intro_video');
         else return "";
+    },
+
+    // Whether or not the course pacing can be toggled. If the course
+    // has already started, returns false; otherwise, returns true.
+    canTogglePace: function () {
+        return new Date() <= new Date(this.get('start_date'));
     }
 });
 

cms/static/js/views/pages/container_subviews.js

@@ -115,7 +115,8 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "common/js/compo
                     releaseDateFrom: this.model.get('release_date_from'),
                     hasExplicitStaffLock: this.model.get('has_explicit_staff_lock'),
                     staffLockFrom: this.model.get('staff_lock_from'),
-                    hasContentGroupComponents: this.model.get('has_content_group_components')
+                    hasContentGroupComponents: this.model.get('has_content_group_components'),
+                    course: window.course,
                 }));
 
                 return this;

cms/static/js/views/settings/main.js

@@ -1,8 +1,9 @@
 define(["js/views/validation", "codemirror", "underscore", "jquery", "jquery.ui", "js/utils/date_utils", "js/models/uploads",
     "js/views/uploads", "js/utils/change_on_enter", "js/views/license", "js/models/license",
-    "common/js/components/views/feedback_notification", "jquery.timepicker", "date"],
-    function(ValidatingView, CodeMirror, _, $, ui, DateUtils, FileUploadModel,
-        FileUploadDialog, TriggerChangeEventOnEnter, LicenseView, LicenseModel, NotificationView) {
+    "common/js/components/views/feedback_notification", "jquery.timepicker", "date", "gettext"],
+       function(ValidatingView, CodeMirror, _, $, ui, DateUtils, FileUploadModel,
+                FileUploadDialog, TriggerChangeEventOnEnter, LicenseView, LicenseModel, NotificationView,
+                timepicker, date, gettext) {
 
 var DetailsView = ValidatingView.extend({
     // Model class is CMS.Models.Settings.CourseDetails
@@ -99,11 +100,19 @@ var DetailsView = ValidatingView.extend({
         }
         this.$('#' + this.fieldToSelectorMap['entrance_exam_minimum_score_pct']).val(this.model.get('entrance_exam_minimum_score_pct'));
 
-        if (this.model.get('self_paced')) {
-            this.$('#course-pace-self-paced').attr('checked', true);
+        var selfPacedButton = this.$('#course-pace-self-paced'),
+            instructorLedButton = this.$('#course-pace-instructor-led'),
+            paceToggleTip = this.$('#course-pace-toggle-tip');
+        (this.model.get('self_paced') ? selfPacedButton : instructorLedButton).attr('checked', true);
+        if (this.model.canTogglePace()) {
+            selfPacedButton.removeAttr('disabled');
+            instructorLedButton.removeAttr('disabled');
+            paceToggleTip.text('');
         }
         else {
-            this.$('#course-pace-instructor-led').attr('checked', true);
+            selfPacedButton.attr('disabled', true);
+            instructorLedButton.attr('disabled', true);
+            paceToggleTip.text(gettext('Course pacing cannot be changed once a course has started.'));
         }
 
         this.licenseView.render()

cms/static/js/views/validation.js

@@ -157,6 +157,7 @@ var ValidatingView = BaseView.extend({
             {
                 success: function() {
                     self.showSavedBar();
+                    self.render();
                 },
                 silent: true
             }

cms/static/sass/views/_settings.scss

@@ -1039,4 +1039,29 @@
       }
     }
   }
+
+  // UI: course pacing options
+  .group-settings.pacing {
+    .list-input {
+      margin-top: $baseline/2;
+      background-color: $gray-l4;
+      border-radius: 3px;
+      padding: ($baseline/2);
+    }
+
+    .field {
+      @include margin(0, 0, $baseline, 0);
+
+      .field-radio {
+        display: inline-block;
+        @include margin-right($baseline/4);
+        width: auto;
+        height: auto;
+
+        & + .course-pace-label {
+          display: inline-block;
+        }
+      }
+    }
+  }
 }

cms/templates/js/publish-xblock.underscore

@@ -42,22 +42,24 @@ var visibleToStaffOnly = visibilityState === 'staff_only';
         </p>
     </div>
 
-    <div class="wrapper-release bar-mod-content">
-        <h5 class="title"><%= releaseLabel %></h5>
-        <p class="copy">
-            <% if (releaseDate) { %>
-                <span class="release-date"><%= releaseDate %></span>
-                <span class="release-with">
-                    <%= interpolate(
-                        gettext('with %(release_date_from)s'), { release_date_from: releaseDateFrom }, true
-                    ) %>
-                </span>
+    <% if (!course.get('self_paced')) { %>
+        <div class="wrapper-release bar-mod-content">
+            <h5 class="title"><%= releaseLabel %></h5>
+            <p class="copy">
+                <% if (releaseDate) { %>
+                    <span class="release-date"><%= releaseDate %></span>
+                    <span class="release-with">
+                        <%= interpolate(
+                            gettext('with %(release_date_from)s'), { release_date_from: releaseDateFrom }, true
+                        ) %>
+                    </span>
 
-            <% } else { %>
-                <%= gettext("Unscheduled") %>
-            <% } %>
-        </p>
-    </div>
+                <% } else { %>
+                    <%= gettext("Unscheduled") %>
+                <% } %>
+            </p>
+        </div>
+    <% } %>
 
     <div class="wrapper-visibility bar-mod-content">
         <h5 class="title">

cms/templates/settings.html

@@ -421,11 +421,20 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url}';
                 <h2 class="title-2">${_("Course Pacing")}</h2>
                 <span class="tip">${_("Set the pacing for this course")}</span>
               </header>
+              <span class="msg" id="course-pace-toggle-tip"></span>
 
-              <input type="radio" name="self-paced" id="course-pace-self-paced" value="true"/>
-              <label for="course-pace-self-paced">Self-Paced</label>
-              <input type="radio" name="self-paced" id="course-pace-instructor-led" value="false"/>
-              <label for="course-pace-instructor-led">Instructor Led</label>
+              <ol class="list-input">
+                <li class="field">
+                  <input type="radio" class="field-radio" name="self-paced" id="course-pace-instructor-led" value="false"/>
+                  <label class="course-pace-label" for="course-pace-instructor-led">Instructor-Led</label>
+                  <span class="tip">${_("Instructor-led courses progress at the pace that the course author sets. You can configure release dates for course content and due dates for assignments.")}</span>
+                </li>
+                <li class="field">
+                  <input type="radio" class="field-radio" name="self-paced" id="course-pace-self-paced" value="true"/>
+                  <label class="course-pace-label" for="course-pace-self-paced">Self-Paced</label>
+                  <span class="tip">${_("Self-paced courses do not have release dates for course content or due dates for assignments. Learners can complete course material at any time before the course end date.")}</span>
+                </li>
+              </ol>
             </section>
 
           % endif

common/lib/xmodule/xmodule/course_module.py

@@ -1584,3 +1584,13 @@ class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin):
             if p.scheme != scheme
         ]
         self.user_partitions = other_partitions + partitions  # pylint: disable=attribute-defined-outside-init
+
+    @property
+    def can_toggle_course_pacing(self):
+        """
+        Whether or not the course can be set to self-paced at this time.
+
+        Returns:
+          bool: False if the course has already started, True otherwise.
+        """
+        return datetime.now(UTC()) <= self.start

common/test/acceptance/pages/studio/settings.py

@@ -131,15 +131,20 @@ class SettingsPage(CoursePage):
             raise Exception("Invalid license name: {name}".format(name=license_name))
         button.click()
 
-    pacing_css = 'section.pacing input[type=radio]:checked'
+    pacing_css = 'section.pacing input[type=radio]'
+
+    @property
+    def checked_pacing_css(self):
+        """CSS for the course pacing button which is currently checked."""
+        return self.pacing_css + ':checked'
 
     @property
     def course_pacing(self):
         """
         Returns the label text corresponding to the checked pacing radio button.
         """
-        self.wait_for_element_presence(self.pacing_css, 'course pacing controls present and rendered')
-        checked = self.q(css=self.pacing_css).results[0]
+        self.wait_for_element_presence(self.checked_pacing_css, 'course pacing controls present and rendered')
+        checked = self.q(css=self.checked_pacing_css).results[0]
         checked_id = checked.get_attribute('id')
         return self.q(css='label[for={checked_id}]'.format(checked_id=checked_id)).results[0].text
 
@@ -149,9 +154,24 @@ class SettingsPage(CoursePage):
         Sets the course to either self-paced or instructor-led by checking
         the appropriate radio button.
         """
-        self.wait_for_element_presence(self.pacing_css, 'course pacing controls present')
+        self.wait_for_element_presence(self.checked_pacing_css, 'course pacing controls present')
         self.q(xpath="//label[contains(text(), '{pacing}')]".format(pacing=pacing)).click()
 
+    @property
+    def course_pacing_disabled_text(self):
+        """
+        Return the message indicating that course pacing cannot be toggled.
+        """
+        return self.q(css='#course-pace-toggle-tip').results[0].text
+
+    def course_pacing_disabled(self):
+        """
+        Return True if the course pacing controls are disabled; False otherwise.
+        """
+        self.wait_for_element_presence(self.checked_pacing_css, 'course pacing controls present')
+        statuses = self.q(css=self.pacing_css).map(lambda e: e.get_attribute('disabled')).results
+        return all((s == 'true' for s in statuses))
+
     ################
     # Waits
     ################

common/test/acceptance/tests/studio/test_studio_outline.py

@@ -1766,7 +1766,10 @@ class SelfPacedOutlineTest(CourseOutlineTest):
                 )
             ),
         )
-        self.course_fixture.add_course_details({'self_paced': True})
+        self.course_fixture.add_course_details({
+            'self_paced': True,
+            'start_date': datetime.now() + timedelta(days=1)
+        })
         ConfigModelFixture('/config/self_paced', {'enabled': True}).install()
 
     def test_release_dates_not_shown(self):

common/test/acceptance/tests/studio/test_studio_settings_details.py

@@ -1,6 +1,7 @@
 """
 Acceptance tests for Studio's Settings Details pages
 """
+from datetime import datetime, timedelta
 from unittest import skip
 
 from .base_studio_test import StudioCourseTest
@@ -205,19 +206,23 @@ class CoursePacingTest(StudioSettingsDetailsTest):
 
     def populate_course_fixture(self, __):
         ConfigModelFixture('/config/self_paced', {'enabled': True}).install()
+        # Set the course start date to tomorrow in order to allow setting pacing
+        self.course_fixture.add_course_details({'start_date': datetime.now() + timedelta(days=1)})
 
     def test_default_instructor_led(self):
         """
         Test that the 'instructor led' button is checked by default.
         """
-        self.assertEqual(self.settings_detail.course_pacing, 'Instructor Led')
+        self.assertEqual(self.settings_detail.course_pacing, 'Instructor-Led')
 
     def test_self_paced(self):
         """
         Test that the 'self-paced' button is checked for a self-paced
         course.
         """
-        self.course_fixture.add_course_details({'self_paced': True})
+        self.course_fixture.add_course_details({
+            'self_paced': True
+        })
         self.course_fixture.configure_course()
         self.settings_detail.refresh_page()
         self.assertEqual(self.settings_detail.course_pacing, 'Self-Paced')
@@ -230,3 +235,14 @@ class CoursePacingTest(StudioSettingsDetailsTest):
         self.settings_detail.save_changes()
         self.settings_detail.refresh_page()
         self.assertEqual(self.settings_detail.course_pacing, 'Self-Paced')
+
+    def test_toggle_pacing_after_course_start(self):
+        """
+        Test that course authors cannot toggle the pacing of their course
+        while the course is running.
+        """
+        self.course_fixture.add_course_details({'start_date': datetime.now()})
+        self.course_fixture.configure_course()
+        self.settings_detail.refresh_page()
+        self.assertTrue(self.settings_detail.course_pacing_disabled())
+        self.assertIn('Course pacing cannot be changed', self.settings_detail.course_pacing_disabled_text)

lms/djangoapps/courseware/self_paced_overrides.py

@@ -14,8 +14,12 @@ class SelfPacedDateOverrideProvider(FieldOverrideProvider):
     due dates to be overridden for self-paced courses.
     """
     def get(self, block, name, default):
+        # Remove due dates
         if name == 'due':
             return None
+        # Remove release dates for course content
+        if name == 'start' and block.category != 'course':
+            return None
         return default
 
     @classmethod

lms/djangoapps/courseware/tests/test_views.py

@@ -6,6 +6,7 @@ import cgi
 from urllib import urlencode
 import ddt
 import json
+import itertools
 import unittest
 from datetime import datetime
 from HTMLParser import HTMLParser
@@ -36,6 +37,7 @@ from courseware.testutils import RenderXBlockTestMixin
 from courseware.tests.factories import StudentModuleFactory
 from courseware.user_state_client import DjangoXBlockUserStateClient
 from edxmako.tests import mako_middleware_process_request
+from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration
 from student.models import CourseEnrollment
 from student.tests.factories import AdminFactory, UserFactory, CourseEnrollmentFactory
 from util.tests.test_date_utils import fake_ugettext, fake_pgettext
@@ -45,7 +47,7 @@ from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
-from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
 
 
 @attr('shard_1')
@@ -669,10 +671,18 @@ class ProgressPageTests(ModuleStoreTestCase):
         self.request.user = self.user
 
         mako_middleware_process_request(self.request)
+
+        self.setup_course()
+
+    def setup_course(self, **options):
+        """Create the test course."""
         course = CourseFactory.create(
             start=datetime(2013, 9, 16, 7, 17, 28),
             grade_cutoffs={u'çü†øƒƒ': 0.75, 'Pass': 0.5},
+            **options
         )
+
+        # pylint: disable=attribute-defined-outside-init
         self.course = modulestore().get_course(course.id)
         CourseEnrollmentFactory(user=self.user, course_id=self.course.id)
 
@@ -829,6 +839,18 @@ class ProgressPageTests(ModuleStoreTestCase):
         resp = views.progress(self.request, course_id=unicode(self.course.id))
         self.assertContains(resp, u"Download Your Certificate")
 
+    @ddt.data(
+        *itertools.product(((18, 4, True), (18, 4, False)), (True, False))
+    )
+    @ddt.unpack
+    def test_query_counts(self, (sql_calls, mongo_calls, self_paced), self_paced_enabled):
+        """Test that query counts remain the same for self-paced and instructor-led courses."""
+        SelfPacedConfiguration(enabled=self_paced_enabled).save()
+        self.setup_course(self_paced=self_paced)
+        with self.assertNumQueries(sql_calls), check_mongo_calls(mongo_calls):
+            resp = views.progress(self.request, course_id=unicode(self.course.id))
+        self.assertEqual(resp.status_code, 200)
+
 
 @attr('shard_1')
 class VerifyCourseKeyDecoratorTests(TestCase):
@@ -1151,3 +1173,17 @@ class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase):
         if url_encoded_params:
             url += '?' + url_encoded_params
         return self.client.get(url)
+
+
+class TestRenderXBlockSelfPaced(TestRenderXBlock):
+    """
+    Test rendering XBlocks for a self-paced course. Relies on the query
+    count assertions in the tests defined by RenderXBlockMixin.
+    """
+
+    def setUp(self):
+        super(TestRenderXBlockSelfPaced, self).setUp()
+        SelfPacedConfiguration(enabled=True).save()
+
+    def course_options(self):
+        return {'self_paced': True}

lms/djangoapps/courseware/testutils.py

@@ -55,6 +55,13 @@ class RenderXBlockTestMixin(object):
         """
         self.client.login(username=self.user.username, password='test')
 
+    def course_options(self):
+        """
+        Options to configure the test course. Intended to be overridden by
+        subclasses.
+        """
+        return {}
+
     def setup_course(self, default_store=None):
         """
         Helper method to create the course.
@@ -62,7 +69,7 @@ class RenderXBlockTestMixin(object):
         if not default_store:
             default_store = self.store.default_modulestore.get_modulestore_type()
         with self.store.default_store(default_store):
-            self.course = CourseFactory.create()  # pylint: disable=attribute-defined-outside-init
+            self.course = CourseFactory.create(**self.course_options())  # pylint: disable=attribute-defined-outside-init
             chapter = ItemFactory.create(parent=self.course, category='chapter')
             self.html_block = ItemFactory.create(  # pylint: disable=attribute-defined-outside-init
                 parent=chapter,