Merge pull request #4059 from edx/andya/xblocks-as-advanced-problems

Allow xblocks to be added as advanced problem types

CHANGELOG.rst

@@ -5,6 +5,8 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Studio: Move Peer Assessment into advanced problems menu.
+
 Blades: Add context-aware video index. BLD-933
 
 Blades: Fix bug with incorrect link format and redirection. BLD-1049

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -140,7 +140,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.check_components_on_page(
             ADVANCED_COMPONENT_TYPES,
             ['Word cloud', 'Annotation', 'Text Annotation', 'Video Annotation', 'Image Annotation',
-             'Open Response Assessment', 'Peer Grading Interface', 'openassessment', 'split_test'],
+             'Open Response Assessment', 'Peer Grading Interface', 'split_test'],
         )
 
     def test_advanced_components_require_two_clicks(self):

cms/djangoapps/contentstore/views/component.py

@@ -45,7 +45,6 @@ COMPONENT_TYPES = ['discussion', 'html', 'problem', 'video']
 
 OPEN_ENDED_COMPONENT_TYPES = ["combinedopenended", "peergrading"]
 NOTE_COMPONENT_TYPES = ['notes']
-
 if settings.FEATURES.get('ALLOW_ALL_ADVANCED_COMPONENTS'):
     ADVANCED_COMPONENT_TYPES = sorted(set(name for name, class_ in XBlock.load_classes()) - set(COMPONENT_TYPES))
 else:
@@ -65,13 +64,20 @@ else:
         'concept',  # Concept mapper. See https://github.com/pmitros/ConceptXBlock
         'done',  # Lets students mark things as done. See https://github.com/pmitros/DoneXBlock
         'audio',  # Embed an audio file. See https://github.com/pmitros/AudioXBlock
-        'openassessment',  # edx-ora2
         'split_test'
     ] + OPEN_ENDED_COMPONENT_TYPES + NOTE_COMPONENT_TYPES
 
 ADVANCED_COMPONENT_CATEGORY = 'advanced'
 ADVANCED_COMPONENT_POLICY_KEY = 'advanced_modules'
 
+# Specify xblocks that should be treated as advanced problems. Each entry is a tuple
+# specifying the xblock name and an optional YAML template to be used.
+ADVANCED_PROBLEM_TYPES = [
+    {
+        'component': 'openassessment',
+        'boilerplate_name': None
+    }
+]
 
 @require_GET
 @login_required
@@ -165,7 +171,7 @@ def unit_handler(request, usage_key_string):
         except ItemNotFoundError:
             return HttpResponseBadRequest()
 
-        component_templates = _get_component_templates(course)
+        component_templates = get_component_templates(course)
 
         xblocks = item.get_children()
 
@@ -245,7 +251,7 @@ def container_handler(request, usage_key_string):
         except ItemNotFoundError:
             return HttpResponseBadRequest()
 
-        component_templates = _get_component_templates(course)
+        component_templates = get_component_templates(course)
         ancestor_xblocks = []
         parent = get_parent_xblock(xblock)
         while parent and parent.category != 'sequential':
@@ -269,7 +275,7 @@ def container_handler(request, usage_key_string):
         return HttpResponseBadRequest("Only supports html requests")
 
 
-def _get_component_templates(course):
+def get_component_templates(course):
     """
     Returns the applicable component templates that can be used by the specified course.
     """
@@ -297,9 +303,19 @@ def _get_component_templates(course):
         'problem': _("Problem"),
         'video': _("Video")
     }
-    advanced_component_display_names = {}
+
+    def get_component_display_name(component, default_display_name=None):
+        """
+        Returns the display name for the specified component.
+        """
+        component_class = _load_mixed_class(component)
+        if hasattr(component_class, 'display_name') and component_class.display_name.default:
+            return _(component_class.display_name.default)
+        else:
+            return default_display_name
 
     component_templates = []
+    categories = set()
     # The component_templates array is in the order of "advanced" (if present), followed
     # by the components in the order listed in COMPONENT_TYPES.
     for category in COMPONENT_TYPES:
@@ -308,11 +324,9 @@ def _get_component_templates(course):
         # add the default template with localized display name
         # TODO: Once mixins are defined per-application, rather than per-runtime,
         # this should use a cms mixed-in class. (cpennington)
-        if hasattr(component_class, 'display_name'):
-            display_name = _(component_class.display_name.default) if component_class.display_name.default else _('Blank')
-        else:
-            display_name = _('Blank')
+        display_name = get_component_display_name(category, _('Blank'))
         templates_for_category.append(create_template_dict(display_name, category))
+        categories.add(category)
 
         # add boilerplates
         if hasattr(component_class, 'templates'):
@@ -327,6 +341,16 @@ def _get_component_templates(course):
                             template['metadata'].get('markdown') is not None
                         )
                     )
+
+        # Add any advanced problem types
+        if category == 'problem':
+            for advanced_problem_type in ADVANCED_PROBLEM_TYPES:
+                component = advanced_problem_type['component']
+                boilerplate_name = advanced_problem_type['boilerplate_name']
+                component_display_name = get_component_display_name(component)
+                templates_for_category.append(create_template_dict(component_display_name, component, boilerplate_name))
+                categories.add(component)
+
         component_templates.append({
             "type": category,
             "templates": templates_for_category,
@@ -342,21 +366,17 @@ def _get_component_templates(course):
     # Set component types according to course policy file
     if isinstance(course_advanced_keys, list):
         for category in course_advanced_keys:
-            if category in ADVANCED_COMPONENT_TYPES:
+            if category in ADVANCED_COMPONENT_TYPES and not category in categories:
                 # boilerplates not supported for advanced components
                 try:
-                    component_class = _load_mixed_class(category)
-
-                    if component_class.display_name.default:
-                        template_display_name = _(component_class.display_name.default)
-                    else:
-                        template_display_name = advanced_component_display_names.get(category, category)
+                    component_display_name = get_component_display_name(category)
                     advanced_component_templates['templates'].append(
                         create_template_dict(
-                            template_display_name,
+                            component_display_name,
                             category
                         )
                     )
+                    categories.add(category)
                 except PluginMissingError:
                     # dhm: I got this once but it can happen any time the
                     # course author configures an advanced component which does

cms/djangoapps/contentstore/views/tests/test_item.py

@@ -14,7 +14,7 @@ from django.test.client import RequestFactory
 from django.core.urlresolvers import reverse
 from contentstore.utils import reverse_usage_url
 
-from contentstore.views.component import component_handler
+from contentstore.views.component import component_handler, get_component_templates
 
 from contentstore.tests.utils import CourseTestCase
 from contentstore.utils import compute_publish_state, PublishState
@@ -947,3 +947,69 @@ class TestComponentHandler(TestCase):
         self.descriptor.handle = create_response
 
         self.assertEquals(component_handler(self.request, self.usage_key_string, 'dummy_handler').status_code, status_code)
+
+
+class TestComponentTemplates(CourseTestCase):
+    """
+    Unit tests for the generation of the component templates for a course.
+    """
+
+    def setUp(self):
+        super(TestComponentTemplates, self).setUp()
+        self.templates = get_component_templates(self.course)
+
+    def get_templates_of_type(self, template_type):
+        """
+        Returns the templates for the specified type, or None if none is found.
+        """
+        template_dict = next((template for template in self.templates if template.get('type') == template_type), None)
+        return template_dict.get('templates') if template_dict else None
+
+    def get_template(self, templates, display_name):
+        """
+        Returns the template which has the specified display name.
+        """
+        return next((template for template in templates if template.get('display_name') == display_name), None)
+
+    def test_basic_components(self):
+        """
+        Test the handling of the basic component templates.
+        """
+        self.assertIsNotNone(self.get_templates_of_type('discussion'))
+        self.assertIsNotNone(self.get_templates_of_type('html'))
+        self.assertIsNotNone(self.get_templates_of_type('problem'))
+        self.assertIsNotNone(self.get_templates_of_type('video'))
+        self.assertIsNone(self.get_templates_of_type('advanced'))
+
+    def test_advanced_components(self):
+        """
+        Test the handling of advanced component templates.
+        """
+        self.course.advanced_modules.append('word_cloud')
+        self.templates = get_component_templates(self.course)
+        advanced_templates = self.get_templates_of_type('advanced')
+        self.assertEqual(len(advanced_templates), 1)
+        world_cloud_template = advanced_templates[0]
+        self.assertEqual(world_cloud_template.get('category'), 'word_cloud')
+        self.assertEqual(world_cloud_template.get('display_name'), u'Word cloud')
+        self.assertIsNone(world_cloud_template.get('boilerplate_name', None))
+
+        # Verify that non-advanced components are not added twice
+        self.course.advanced_modules.append('video')
+        self.course.advanced_modules.append('openassessment')
+        self.templates = get_component_templates(self.course)
+        advanced_templates = self.get_templates_of_type('advanced')
+        self.assertEqual(len(advanced_templates), 1)
+        only_template = advanced_templates[0]
+        self.assertNotEqual(only_template.get('category'), 'video')
+        self.assertNotEqual(only_template.get('category'), 'openassessment')
+
+    def test_advanced_problems(self):
+        """
+        Test the handling of advanced problem templates.
+        """
+        problem_templates = self.get_templates_of_type('problem')
+        ora_template = self.get_template(problem_templates, u'Peer Assessment')
+        self.assertIsNotNone(ora_template)
+        self.assertEqual(ora_template.get('category'), 'openassessment')
+        self.assertIsNone(ora_template.get('boilerplate_name', None))

cms/static/coffee/spec/main.coffee

@@ -212,6 +212,7 @@ define([
     "js/spec/video/transcripts/videolist_spec", "js/spec/video/transcripts/message_manager_spec",
     "js/spec/video/transcripts/file_uploader_spec",
 
+    "js/spec/models/component_template_spec",
     "js/spec/models/explicit_url_spec",
 
     "js/spec/utils/drag_and_drop_spec",

cms/static/js/models/component_template.js

@@ -13,19 +13,31 @@ define(["backbone"], function (Backbone) {
             templates: []
         },
         parse: function (response) {
+            // Returns true only for templates that both have no boilerplate and are of
+            // the overall type of the menu. This allows other component types to be added
+            // and they will get sorted alphabetically rather than just at the top.
+            // e.g. The ORA openassessment xblock is listed as an advanced problem.
+            var isPrimaryBlankTemplate = function(template) {
+                return !template.boilerplate_name && template.category === response.type;
+            };
+
             this.type = response.type;
             this.templates = response.templates;
             this.display_name = response.display_name;
 
             // Sort the templates.
             this.templates.sort(function (a, b) {
-                // The entry without a boilerplate always goes first
-                if (!a.boilerplate_name || (a.display_name < b.display_name)) {
+                // The blank problem for the current type goes first
+                if (isPrimaryBlankTemplate(a)) {
+                    return -1;
+                } else if (isPrimaryBlankTemplate(b)) {
+                    return 1;
+                } else if (a.display_name > b.display_name) {
+                    return 1;
+                } else if (a.display_name < b.display_name) {
                     return -1;
                 }
-                else {
-                    return (a.display_name > b.display_name) ? 1 : 0;
-                }
+                return 0;
             });
         }
     });

cms/static/js/spec/models/component_template_spec.js

+define(["js/models/component_template"],
+    function (ComponentTemplate) {
+
+        describe("ComponentTemplates", function() {
+            var mockTemplateJSON = {
+                "templates": [
+                    {
+                        "category": "problem",
+                        "boilerplate_name": "formularesponse.yaml",
+                        "display_name": "Math Expression Input"
+                    }, {
+                        "category": "problem",
+                        "boilerplate_name": null,
+                        "display_name": "Blank Advanced Problem"
+                    }, {
+                        "category": "problem",
+                        "boilerplate_name": "checkboxes.yaml",
+                        "display_name": "Checkboxes"
+                    }, {
+                        "category": "problem",
+                        "boilerplate_name": "multiple_choice.yaml",
+                        "display_name": "Multiple Choice"
+                    }, {
+                        "category": "problem",
+                        "boilerplate_name": "drag_and_drop.yaml",
+                        "display_name": "Drag and Drop"
+                    }, {
+                        "category": "problem",
+                        "boilerplate_name": "problem_with_hint.yaml",
+                        "display_name": "Problem with Adaptive Hint"
+                    }, {
+                        "category": "problem",
+                        "boilerplate_name": "imageresponse.yaml",
+                        "display_name": "Image Mapped Input"
+                    }, {
+                        "category": "openassessment",
+                        "boilerplate_name": null,
+                        "display_name": "Peer Assessment"
+                    }, {
+                        "category": "problem",
+                        "boilerplate_name": "an_easy_problem.yaml",
+                        "display_name": "An Easy Problem"
+                    }, {
+                        "category": "word_cloud",
+                        "boilerplate_name": null,
+                        "display_name": "Word Cloud"
+                    }, { // duplicate display name to verify sort behavior
+                        "category": "word_cloud",
+                        "boilerplate_name": "alternate_word_cloud.yaml",
+                        "display_name": "Word Cloud"
+                    }],
+                "type": "problem"
+            };
+
+            it('orders templates correctly', function () {
+                var lastTemplate = null,
+                    firstComparison = true,
+                    componentTemplate = new ComponentTemplate(),
+                    template, templateName, i;
+                componentTemplate.parse(mockTemplateJSON);
+                for (i=0; i < componentTemplate.templates.length; i++) {
+                    template = componentTemplate.templates[i];
+                    templateName = template['display_name'];
+                    if (lastTemplate) {
+                        if (!firstComparison || lastTemplate['boilerplate_name']) {
+                            expect(lastTemplate['display_name'] < templateName).toBeTruthy();
+                        }
+                        firstComparison = false;
+                    } else {
+                        // If the first template is blank, make sure that it has the correct category
+                        if (!template['boilerplate_name']) {
+                            expect(template['category']).toBe('problem');
+                        }
+                        lastTemplate = template;
+                    }
+                }
+            });
+        });
+    });