Merge pull request #103 from open-craft/ensure-default-theme

Ensure default theme is used if theme has not been customized

circle.yml

@@ -15,7 +15,7 @@ test:
   override:
     - "if [ $CIRCLE_NODE_INDEX = '0' ]; then pep8 problem_builder --max-line-length=120; fi":
         parallel: true
-    - "if [ $CIRCLE_NODE_INDEX = '1' ]; then pylint problem_builder --disable=all --enable=function-redefined,undefined-variable,unused-variable; fi":
+    - "if [ $CIRCLE_NODE_INDEX = '1' ]; then pylint problem_builder --disable=all --enable=function-redefined,undefined-variable,unused-import,unused-variable; fi":
         parallel: true
     - "python run_tests.py":
         parallel: true

problem_builder/answer.py

@@ -26,7 +26,7 @@ from lazy import lazy
 from .models import Answer
 
 from xblock.core import XBlock
-from xblock.fields import Scope, Float, Integer, String
+from xblock.fields import Scope, Integer, String
 from xblock.fragment import Fragment
 from xblock.validation import ValidationMessage
 from xblockutils.resources import ResourceLoader

problem_builder/mentoring.py

@@ -59,11 +59,6 @@ except ImportError:
 log = logging.getLogger(__name__)
 loader = ResourceLoader(__name__)
 
-_default_theme_config = {
-    'package': 'problem_builder',
-    'locations': ['public/themes/lms.css']
-}
-
 _default_options_config = {
     'pb_mcq_hide_previous_answer': False
 }
@@ -126,9 +121,13 @@ class BaseMentoringBlock(
 
     icon_class = 'problem'
     block_settings_key = 'mentoring'
-    theme_key = 'theme'
     options_key = 'options'
 
+    default_theme_config = {
+        'package': 'problem_builder',
+        'locations': ['public/themes/lms.css']
+    }
+
     @property
     def url_name(self):
         """
@@ -170,7 +169,7 @@ class BaseMentoringBlock(
         Fall back on default options if xblock settings have not been customized at all
         or no customizations for options available.
         """
-        xblock_settings = self.get_xblock_settings(_default_options_config)
+        xblock_settings = self.get_xblock_settings(default={})
         if xblock_settings and self.options_key in xblock_settings:
             return xblock_settings[self.options_key]
         return _default_options_config

problem_builder/south_migrations/0003_auto__add_share__add_unique_share_shared_by_shared_with_block_id.py

 # -*- coding: utf-8 -*-
-from south.utils import datetime_utils as datetime
 from south.db import db
 from south.v2 import SchemaMigration
-from django.db import models
 
 
 class Migration(SchemaMigration):

problem_builder/table.py

@@ -26,7 +26,7 @@ from django.contrib.auth.models import User
 
 from xblock.core import XBlock
 from xblock.exceptions import JsonHandlerError
-from xblock.fields import Scope, String, Boolean, Dict
+from xblock.fields import Scope, String, Boolean
 from xblock.fragment import Fragment
 
 from xblockutils.resources import ResourceLoader

problem_builder/tests/integration/test_assessment.py

@@ -17,7 +17,7 @@
 # along with this program in a file in the toplevel directory called
 # "AGPLv3".  If not, see <http://www.gnu.org/licenses/>.
 #
-from ddt import ddt, unpack, data
+from ddt import ddt, data
 from .base_test import CORRECT, INCORRECT, PARTIAL, MentoringAssessmentBaseTest, GetChoices
 
 

problem_builder/tests/integration/test_author_changes.py

@@ -3,8 +3,8 @@ If an author makes changes to the block after students have started using it, wi
 happen?
 """
 import time
+
 from .base_test import ProblemBuilderBaseTest, MentoringAssessmentBaseTest
-import re
 
 
 class AuthorChangesTest(ProblemBuilderBaseTest):

problem_builder/tests/integration/test_dashboard.py

@@ -21,7 +21,6 @@ from textwrap import dedent
 from mock import Mock, patch
 from .base_test import ProblemBuilderBaseTest
 from selenium.common.exceptions import NoSuchElementException
-from xblockutils.resources import ResourceLoader
 
 
 class MockSubmissionsAPI(object):

problem_builder/tests/unit/test_dashboard_visual.py

 """
 Unit tests for DashboardVisualData
 """
-from problem_builder.dashboard_visual import DashboardVisualData
-from mock import MagicMock, Mock
 import unittest
-from xblock.field_data import DictFieldData
+
+from problem_builder.dashboard_visual import DashboardVisualData
 
 
 class TestDashboardVisualData(unittest.TestCase):

problem_builder/tests/unit/test_problem_builder.py

@@ -7,9 +7,7 @@ from random import random
 from xblock.field_data import DictFieldData
 
 from problem_builder.mcq import MCQBlock
-from problem_builder.mentoring import (
-    MentoringBlock, MentoringMessageBlock, _default_theme_config, _default_options_config
-)
+from problem_builder.mentoring import MentoringBlock, MentoringMessageBlock, _default_options_config
 
 
 @ddt.ddt
@@ -69,6 +67,7 @@ class TestMentoringBlock(unittest.TestCase):
             self.assertIn('Unable to load child component', fragment.content)
 
 
+@ddt.ddt
 class TestMentoringBlockTheming(unittest.TestCase):
     def setUp(self):
         self.service_mock = Mock()
@@ -76,6 +75,70 @@ class TestMentoringBlockTheming(unittest.TestCase):
         self.runtime_mock.service = Mock(return_value=self.service_mock)
         self.block = MentoringBlock(self.runtime_mock, DictFieldData({}), Mock())
 
+    def test_get_theme_returns_default_if_settings_service_is_not_available(self):
+        self.runtime_mock.service = Mock(return_value=None)
+        theme = self.block.get_theme()
+        # Ensure MentoringBlock overrides "default_theme_config" from ThemableXBlockMixin with meaningful value:
+        self.assertIsNotNone(theme)
+        self.assertEqual(theme, MentoringBlock.default_theme_config)
+
+    def test_get_theme_returns_default_if_xblock_settings_not_customized(self):
+        self.block.get_xblock_settings = Mock(return_value=None)
+        theme = self.block.get_theme()
+        # Ensure MentoringBlock overrides "default_theme_config" from ThemableXBlockMixin with meaningful value:
+        self.assertIsNotNone(theme)
+        self.assertEqual(theme, MentoringBlock.default_theme_config)
+        self.block.get_xblock_settings.assert_called_once_with(default={})
+
+    @ddt.data(
+        {}, {'mass': 123}, {'spin': {}}, {'parity': "1"}
+    )
+    def test_get_theme_returns_default_if_theme_not_customized(self, xblock_settings):
+        self.block.get_xblock_settings = Mock(return_value=xblock_settings)
+        theme = self.block.get_theme()
+        # Ensure MentoringBlock overrides "default_theme_config" from ThemableXBlockMixin with meaningful value:
+        self.assertIsNotNone(theme)
+        self.assertEqual(theme, MentoringBlock.default_theme_config)
+        self.block.get_xblock_settings.assert_called_once_with(default={})
+
+    @ddt.data(
+        {MentoringBlock.theme_key: 123},
+        {MentoringBlock.theme_key: [1, 2, 3]},
+        {MentoringBlock.theme_key: {'package': 'qwerty', 'locations': ['something_else.css']}},
+    )
+    def test_get_theme_correctly_returns_customized_theme(self, xblock_settings):
+        self.block.get_xblock_settings = Mock(return_value=xblock_settings)
+        theme = self.block.get_theme()
+        # Ensure MentoringBlock overrides "default_theme_config" from ThemableXBlockMixin with meaningful value:
+        self.assertIsNotNone(theme)
+        self.assertEqual(theme, xblock_settings[MentoringBlock.theme_key])
+        self.block.get_xblock_settings.assert_called_once_with(default={})
+
+    def test_theme_files_are_loaded_from_correct_package(self):
+        fragment = MagicMock()
+        package_name = 'some_package'
+        xblock_settings = {MentoringBlock.theme_key: {'package': package_name, 'locations': ['lms.css']}}
+        self.block.get_xblock_settings = Mock(return_value=xblock_settings)
+        with patch("xblockutils.settings.ResourceLoader") as patched_resource_loader:
+            self.block.include_theme_files(fragment)
+            patched_resource_loader.assert_called_with(package_name)
+
+    @ddt.data(
+        ('problem_builder', ['public/themes/lms.css']),
+        ('problem_builder', ['public/themes/lms.css', 'public/themes/lms.part2.css']),
+        ('my_app.my_rules', ['typography.css', 'icons.css']),
+    )
+    @ddt.unpack
+    def test_theme_files_are_added_to_fragment(self, package_name, locations):
+        fragment = MagicMock()
+        xblock_settings = {MentoringBlock.theme_key: {'package': package_name, 'locations': locations}}
+        self.block.get_xblock_settings = Mock(return_value=xblock_settings)
+        with patch("xblockutils.settings.ResourceLoader.load_unicode") as patched_load_unicode:
+            self.block.include_theme_files(fragment)
+            for location in locations:
+                patched_load_unicode.assert_any_call(location)
+            self.assertEqual(patched_load_unicode.call_count, len(locations))
+
     def test_student_view_calls_include_theme_files(self):
         self.service_mock.get_settings_bucket = Mock(return_value={})
         with patch.object(self.block, 'include_theme_files') as patched_include_theme_files:
@@ -97,10 +160,14 @@ class TestMentoringBlockOptions(unittest.TestCase):
         self.runtime_mock.service = Mock(return_value=self.service_mock)
         self.block = MentoringBlock(self.runtime_mock, DictFieldData({}), Mock())
 
+    def test_get_options_returns_default_if_settings_service_is_not_available(self):
+        self.runtime_mock.service = Mock(return_value=None)
+        self.assertEqual(self.block.get_options(), _default_options_config)
+
     def test_get_options_returns_default_if_xblock_settings_not_customized(self):
         self.block.get_xblock_settings = Mock(return_value=None)
         self.assertEqual(self.block.get_options(), _default_options_config)
-        self.block.get_xblock_settings.assert_called_once_with(_default_options_config)
+        self.block.get_xblock_settings.assert_called_once_with(default={})
 
     @ddt.data(
         {}, {'mass': 123}, {'spin': {}}, {'parity': "1"}
@@ -108,7 +175,7 @@ class TestMentoringBlockOptions(unittest.TestCase):
     def test_get_options_returns_default_if_options_not_customized(self, xblock_settings):
         self.block.get_xblock_settings = Mock(return_value=xblock_settings)
         self.assertEqual(self.block.get_options(), _default_options_config)
-        self.block.get_xblock_settings.assert_called_once_with(_default_options_config)
+        self.block.get_xblock_settings.assert_called_once_with(default={})
 
     @ddt.data(
         {MentoringBlock.options_key: 123},
@@ -118,7 +185,7 @@ class TestMentoringBlockOptions(unittest.TestCase):
     def test_get_options_correctly_returns_customized_options(self, xblock_settings):
         self.block.get_xblock_settings = Mock(return_value=xblock_settings)
         self.assertEqual(self.block.get_options(), xblock_settings[MentoringBlock.options_key])
-        self.block.get_xblock_settings.assert_called_once_with(_default_options_config)
+        self.block.get_xblock_settings.assert_called_once_with(default={})
 
     def test_get_option(self):
         random_key, random_value = random(), random()