Merge pull request #11202 from edx/release

Release for Jan 12, 2016

common/djangoapps/static_replace/migrations/0001_initial.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+from django.conf import settings
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='AssetBaseUrlConfig',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')),
+                ('enabled', models.BooleanField(default=False, verbose_name='Enabled')),
+                ('base_url', models.TextField(help_text=b'The alternative hostname to serve static assets from.  Should be in the form of hostname[:port].', blank=True)),
+                ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')),
+            ],
+        ),
+    ]

lms/static/coffee/fixtures/calculator.html

@@ -7,6 +7,18 @@
           <input type="text" id="calculator_input" tabindex="-1" />
           <div class="help-wrapper">
             <a id="calculator_hint" href="#" role="button" aria-haspopup="true" tabindex="-1">Hints</a>
+            <ul id="calculator_input_help" class="help" aria-activedescendant="hint-integers" role="tooltip" aria-hidden="true">
+                <li class="hint-item" id="hint-moreinfo" tabindex="-1">
+                  <p>
+                    <span class="bold">For detailed information, see
+                      <a id="hint-link-first" href="http://edx-guide-for-students.readthedocs.org/en/latest/SFD_mathformatting.html">Entering Mathematical and Scientific Expressions</a>in the
+                      <a id="hint-link-second" href="http://edx-guide-for-students.readthedocs.org/en/latest/index.html">edX Guide for Students </a> format
+                    </span>
+                  </p>
+                </li>
+                <li class="hint-item" id="hint-integers" tabindex="-1"><p><span class="bold">Integers:</span> 2520</p></li>
+                <li class="hint-item" id="hint-decimals" tabindex="-1"><p><span class="bold">Decimals:</span> 3.14 or .98</p></li>
+            </ul>
           </div>
         </div>
         <input id="calculator_button" type="submit" title="Calculate" arial-label="Calculate" value="=" tabindex="-1" />
@@ -14,18 +26,4 @@
       </form>
     </div>
   </li>
-  <li>
-    <ul id="calculator_input_help" class="calc-help-popup" aria-activedescendant="hint-integers" role="tooltip" aria-hidden="true">
-        <li class="hint-item" id="hint-moreinfo" tabindex="-1">
-          <p>
-            <span class="bold">For detailed information, see
-              <a id="hint-link-first" href="http://edx-guide-for-students.readthedocs.org/en/latest/SFD_mathformatting.html">Entering Mathematical and Scientific Expressions</a>in the
-              <a id="hint-link-second" href="http://edx-guide-for-students.readthedocs.org/en/latest/index.html">edX Guide for Students </a> format
-            </span>
-          </p>
-        </li>
-        <li class="hint-item" id="hint-integers" tabindex="-1"><p><span class="bold">Integers:</span> 2520</p></li>
-        <li class="hint-item" id="hint-decimals" tabindex="-1"><p><span class="bold">Decimals:</span> 3.14 or .98</p></li>
-    </ul>
-  </li>
 </ul>

lms/static/coffee/spec/calculator_spec.coffee

@@ -61,35 +61,35 @@ describe 'Calculator', ->
   describe 'showHint', ->
     it 'show the help overlay', ->
       @calculator.showHint()
-      expect($('.calc-help-popup')).toHaveClass('shown')
-      expect($('.calc-help-popup')).toHaveAttr('aria-hidden', 'false')
+      expect($('.help')).toHaveClass('shown')
+      expect($('.help')).toHaveAttr('aria-hidden', 'false')
 
 
   describe 'hideHint', ->
     it 'show the help overlay', ->
       @calculator.hideHint()
-      expect($('.calc-help-popup')).not.toHaveClass('shown')
-      expect($('.calc-help-popup')).toHaveAttr('aria-hidden', 'true')
+      expect($('.help')).not.toHaveClass('shown')
+      expect($('.help')).toHaveAttr('aria-hidden', 'true')
 
   describe 'handleClickOnHintButton', ->
     it 'on click hint button hint popup becomes visible ', ->
       e = jQuery.Event('click');
       $('#calculator_hint').trigger(e);
-      expect($('.calc-help-popup')).toHaveClass 'shown'
+      expect($('.help')).toHaveClass 'shown'
 
   describe 'handleClickOnDocument', ->
     it 'on click out of the hint popup it becomes hidden', ->
       @calculator.showHint()
       e = jQuery.Event('click');
       $(document).trigger(e);
-      expect($('.calc-help-popup')).not.toHaveClass 'shown'
+      expect($('.help')).not.toHaveClass 'shown'
 
   describe 'handleClickOnHintPopup', ->
     it 'on click of hint popup it remains visible', ->
       @calculator.showHint()
       e = jQuery.Event('click');
       $('#calculator_input_help').trigger(e);
-      expect($('.calc-help-popup')).toHaveClass 'shown'
+      expect($('.help')).toHaveClass 'shown'
 
   describe 'selectHint', ->
     it 'select correct hint item', ->

lms/static/coffee/src/calculator.coffee

@@ -14,7 +14,7 @@
 class @Calculator
   constructor: ->
     @hintButton = $('#calculator_hint')
-    @hintPopup = $('.calc-help-popup')
+    @hintPopup = $('.help')
     @hintsList = @hintPopup.find('.hint-item')
     @selectHint($('#' + @hintPopup.attr('aria-activedescendant')));
 

lms/static/sass/course/modules/_calculator.scss

@@ -10,7 +10,6 @@
   @include transition(bottom $tmg-avg linear 0s);
   -webkit-appearance: none;
   width: 100%;
-  direction: ltr;
 
   &.open {
     bottom: -36px;
@@ -53,6 +52,17 @@
     background: $black;
     color: $white;
 
+    // UI: input help table
+    .calculator-input-help-table {
+      margin: ($baseline/2) 0;
+
+      tr th, tr td {
+        vertical-align: top;
+        border: 1px solid $gray-l4;
+        padding: ($baseline/2);
+      }
+    }
+
     form {
       @extend .clearfix;
       @include box-sizing(border-box);
@@ -147,7 +157,6 @@
             border: none;
             background: url("#{$static-path}/images/info-icon.png") center center no-repeat;
             color: $white;
-            cursor: pointer;
 
             &:focus {
               outline: 5px auto #5b9dd9;
@@ -160,85 +169,47 @@
               color: $white;
             }
           }
-        }
-      }
-    }
-  }
-}
-
-.calc-help-popup {
-  @include transition(none);
 
-  position: fixed;
-  left: 180px;
-  right: 180px;
-  top: 70px;
-  bottom: 90px;
-  overflow: auto;
-
-  background: $white;
-  border-radius: 3px;
-  box-shadow: // Standard 3px black shadow.
-              0 0 3px #999,
-              // Places a overlay black transparent popup background.
-              0 0 0 4000em rgba(0, 0, 0, 0.20);
-
-  color: #333;
-  padding: 10px;
-
-  pointer-events: none;
-  display: none;
-  margin: 0;
-  list-style: none;
-
-
-
-  // UI: input help table
-  .calculator-input-help-table {
-    margin: ($baseline/2) auto;
-
-    tr th, tr td {
-      vertical-align: top;
-      border: 1px solid $gray-l4;
-      padding: ($baseline/2);
-    }
-  }
-
-  &.shown {
-    display: block;
-    pointer-events: auto;
-  }
+          .help {
+            @include transition(none);
+            background: $white;
+            border-radius: 3px;
+            box-shadow: 0 0 3px #999;
+            color: #333;
+            padding: 10px;
+            position: absolute;
+            right: -40px;
+            bottom: 57px;
+            width: 600px;
+            overflow: hidden;
+            pointer-events: none;
+            display: none;
+            margin: 0;
+            list-style: none;
+
+            &.shown {
+              display: block;
+              pointer-events: auto;
+            }
 
-  .bold {
-    font-weight: bold;
-  }
+            .bold {
+              font-weight: bold;
+            }
 
-  p, p+p {
-    margin: 0;
-  }
+            p, p+p {
+              margin: 0;
+            }
 
-  .calc-postfixes {
-    margin: 10px auto;
+            .calc-postfixes {
+              margin: 10px auto;
 
-    td, th {
-      padding: 2px 15px;
+              td, th {
+                padding: 2px 15px;
+              }
+            }
+          }
+        }
+      }
     }
   }
-
-  @media screen and (max-width: 990px) {
-    left: 20px;
-    right: 20px;
-  }
-
-  @media screen and (min-width: 991px) and (max-width: 1999px) {
-    left: 20%;
-    right: 20%;
-  }
-
-  @media screen and (min-width: 2000px) {
-    left: 35%;
-    right: 35%;
-    padding-bottom: 40px;
-    bottom: auto;
-  }
 }

openedx/core/djangoapps/content/course_overviews/models.py

@@ -680,8 +680,9 @@ class CourseOverviewImageSet(TimeStampedModel):
         # an error or the course has no source course_image), our url fields
         # just keep their blank defaults.
         try:
-            image_set.save()
-            course_overview.image_set = image_set
+            with transaction.atomic():
+                image_set.save()
+                course_overview.image_set = image_set
         except (IntegrityError, ValueError):
             # In the event of a race condition that tries to save two image sets
             # to the same CourseOverview, we'll just silently pass on the one

openedx/core/djangoapps/content/course_overviews/tests.py

@@ -32,7 +32,7 @@ from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls, check_mongo_calls_range
 
-from .models import CourseOverview, CourseOverviewImageConfig
+from .models import CourseOverview, CourseOverviewImageSet, CourseOverviewImageConfig
 
 
 @ddt.ddt
@@ -494,6 +494,27 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
             {c.id for c in org_courses[1]},
         )
 
+    def test_get_all_courses_by_mobile_available(self):
+        non_mobile_course = CourseFactory.create(emit_signals=True)
+        mobile_course = CourseFactory.create(mobile_available=True, emit_signals=True)
+
+        test_cases = (
+            (None, {non_mobile_course.id, mobile_course.id}),
+            (dict(mobile_available=True), {mobile_course.id}),
+            (dict(mobile_available=False), {non_mobile_course.id}),
+        )
+
+        for filter_, expected_courses in test_cases:
+            self.assertEqual(
+                {
+                    course_overview.id
+                    for course_overview in
+                    CourseOverview.get_all_courses(filter_=filter_)
+                },
+                expected_courses,
+                "testing CourseOverview.get_all_courses with filter_={}".format(filter_),
+            )
+
 
 @ddt.ddt
 class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
@@ -797,6 +818,43 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
                 self.assertEqual(src_x, image_x)
                 self.assertEqual(src_y, image_y)
 
+    def test_image_creation_race_condition(self):
+        """
+        Test for race condition in CourseOverviewImageSet creation.
+
+        CourseOverviewTestCase already tests for race conditions with
+        CourseOverview as a whole, but we still need to test the case where a
+        CourseOverview already exists and we have a race condition purely in the
+        part that adds a new CourseOverviewImageSet.
+        """
+        # Set config to False so that we don't create the image yet
+        self.set_config(False)
+        course = CourseFactory.create()
+
+        # First create our CourseOverview
+        overview = CourseOverview.get_from_id(course.id)
+        self.assertFalse(hasattr(overview, 'image_set'))
+
+        # Now create an ImageSet by hand...
+        CourseOverviewImageSet.objects.create(course_overview=overview)
+
+        # Now do it the normal way -- this will cause an IntegrityError to be
+        # thrown and suppressed in create_for_course()
+        self.set_config(True)
+        CourseOverviewImageSet.create_for_course(overview)
+        self.assertTrue(hasattr(overview, 'image_set'))
+
+        # The following is actually very important for this test because
+        # set_config() does a model insert after create_for_course() has caught
+        # and supressed an IntegrityError above. If create_for_course() properly
+        # wraps that operation in a transaction.atomic() block, the following
+        # will execute fine. If create_for_course() doesn't use an atomic block,
+        # the following line will cause a TransactionManagementError because
+        # Django will detect that something has already been rolled back in this
+        # transaction. So we don't really care about setting the config -- it's
+        # just a convenient way to cause a database write operation to happen.
+        self.set_config(False)
+
     def _assert_image_urls_all_default(self, modulestore_type, raw_course_image_name, expected_url=None):
         """
         Helper for asserting that all image_urls are defaulting to a particular value.
@@ -826,24 +884,3 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
                 }
             )
             return course_overview
-
-    def test_get_all_courses_by_mobile_available(self):
-        non_mobile_course = CourseFactory.create(emit_signals=True)
-        mobile_course = CourseFactory.create(mobile_available=True, emit_signals=True)
-
-        test_cases = (
-            (None, {non_mobile_course.id, mobile_course.id}),
-            (dict(mobile_available=True), {mobile_course.id}),
-            (dict(mobile_available=False), {non_mobile_course.id}),
-        )
-
-        for filter_, expected_courses in test_cases:
-            self.assertEqual(
-                {
-                    course_overview.id
-                    for course_overview in
-                    CourseOverview.get_all_courses(filter_=filter_)
-                },
-                expected_courses,
-                "testing CourseOverview.get_all_courses with filter_={}".format(filter_),
-            )