Merge pull request #5881 from edx/christina/notification-requirejs

Use factory file so JS used by studio_xblock_wrapper.html goes through RequireJS Optimizer

cms/static/build.js

@@ -45,7 +45,8 @@
             'js/factories/settings',
             'js/factories/settings_advanced',
             'js/factories/settings_graders',
-            'js/factories/textbooks'
+            'js/factories/textbooks',
+            'js/factories/xblock_validation'
         ]),
         /**
          * By default all the configuration for optimization happens from the command

cms/static/coffee/spec/main.coffee

@@ -244,6 +244,8 @@ define([
     "js/spec/views/modals/edit_xblock_spec",
     "js/spec/views/modals/validation_error_modal_spec",
 
+    "js/spec/factories/xblock_validation_spec",
+
     "js/spec/xblock/cms.runtime.v1_spec",
 
     # these tests are run separately in the cms-squire suite, due to process

cms/static/js/factories/xblock_validation.js

+define(["js/views/xblock_validation", "js/models/xblock_validation"],
+function (XBlockValidationView, XBlockValidationModel) {
+    'use strict';
+    return function (validationMessages, hasEditingUrl, isRoot, validationEle) {
+        if (hasEditingUrl && !isRoot) {
+            validationMessages.showSummaryOnly = true;
+        }
+
+        var model = new XBlockValidationModel(validationMessages, {parse: true});
+
+        if (!model.get("empty")) {
+            new XBlockValidationView({el: validationEle, model: model, root: isRoot}).render();
+        }
+    };
+});

cms/static/js/models/xblock_validation.js

@@ -18,10 +18,10 @@ define(["backbone", "gettext", "underscore"], function (Backbone, gettext, _) {
             if (!response.empty) {
                 var summary = "summary" in response ? response.summary : {};
                 var messages = "messages" in response ? response.messages : [];
-                if (!(_.has(summary, "text")) || !summary.text) {
+                if (!summary.text) {
                     summary.text = gettext("This component has validation issues.");
                 }
-                if (!(_.has(summary, "type")) || !summary.type) {
+                if (!summary.type) {
                     summary.type = this.WARNING;
                     // Possible types are ERROR, WARNING, and NOT_CONFIGURED. NOT_CONFIGURED is treated as a warning.
                     _.find(messages, function (message) {

cms/static/js/spec/factories/xblock_validation_spec.js

+define(['jquery', 'js/factories/xblock_validation', 'js/common_helpers/template_helpers'],
+    function($, XBlockValidationFactory, TemplateHelpers) {
+
+        describe('XBlockValidationFactory', function() {
+            var messageDiv;
+
+            beforeEach(function () {
+                TemplateHelpers.installTemplate('xblock-validation-messages');
+                appendSetFixtures($('<div class="messages"></div>'));
+                messageDiv = $('.messages');
+            });
+
+            it('Does not attach a view if messages is empty', function() {
+                XBlockValidationFactory({"empty": true}, false, false, messageDiv);
+                expect(messageDiv.children().length).toEqual(0);
+            });
+
+            it('Does attach a view if messages are not empty', function() {
+                XBlockValidationFactory({"empty": false}, false, false, messageDiv);
+                expect(messageDiv.children().length).toEqual(1);
+            });
+
+            it('Passes through the root property to the view.', function() {
+                var noContainerContent = "no-container-content";
+
+                var notConfiguredMessages = {
+                    "empty": false,
+                    "summary": {"text": "my summary", "type": "not-configured"},
+                    "messages": [],
+                    "xblock_id": "id"
+                };
+                // Root is false, will not add noContainerContent.
+                XBlockValidationFactory(notConfiguredMessages, true, false, messageDiv);
+                expect(messageDiv.find('.validation')).not.toHaveClass(noContainerContent);
+
+                // Root is true, will add noContainerContent.
+                XBlockValidationFactory(notConfiguredMessages, true, true, messageDiv);
+                expect(messageDiv.find('.validation')).toHaveClass(noContainerContent);
+            });
+
+            describe('Controls display of detailed messages based on url and root property', function() {
+                var messagesWithSummary, checkDetailedMessages;
+
+                beforeEach(function () {
+                    messagesWithSummary = {
+                        "empty": false,
+                        "summary": {"text": "my summary"},
+                        "messages": [{"text": "one", "type": "warning"}, {"text": "two", "type": "error"}],
+                        "xblock_id": "id"
+                    };
+                });
+
+                checkDetailedMessages = function (expectedDetailedMessages) {
+                    expect(messageDiv.children().length).toEqual(1);
+                    expect(messageDiv.find('.xblock-message-item').length).toBe(expectedDetailedMessages);
+                };
+
+                it('Does not show details if xblock has an editing URL and it is not rendered as root', function() {
+                    XBlockValidationFactory(messagesWithSummary, true, false, messageDiv);
+                    checkDetailedMessages(0);
+                });
+
+                it('Shows details if xblock does not have its own editing URL, regardless of root value', function() {
+                    XBlockValidationFactory(messagesWithSummary, false, false, messageDiv);
+                    checkDetailedMessages(2);
+
+                    XBlockValidationFactory(messagesWithSummary, false, true, messageDiv);
+                    checkDetailedMessages(2);
+                });
+
+                it('Shows details if xblock has its own editing URL and is rendered as root', function() {
+                    XBlockValidationFactory(messagesWithSummary, true, true, messageDiv);
+                    checkDetailedMessages(2);
+                });
+            });
+        });
+    }
+);

cms/templates/studio_xblock_wrapper.html

-
 <%!
 from django.utils.translation import ugettext as _
 from contentstore.views.helpers import xblock_studio_url
@@ -21,31 +20,17 @@ messages = json.dumps(xblock.validate().to_json())
 </script>
 </%block>
 
-<script type='text/javascript'>
-    require(["js/views/xblock_validation", "js/models/xblock_validation"],
-            function (XBlockValidationView, XBlockValidationModel) {
-                var validationMessages = ${messages};
-                % if xblock_url and not is_root:
-                validationMessages.showSummaryOnly = true;
-                % endif
-
-                var model = new XBlockValidationModel(validationMessages, {parse: true});
-                if (!model.get("empty")) {
-                    var validationEle = $('div.xblock-validation-messages[data-locator="${xblock.location | h}"]');
-                    var viewOptions = {
-                        el: validationEle,
-                        model: model
-                    };
-                    % if is_root:
-                    viewOptions.root = true;
-                    % endif
-                    var view = new XBlockValidationView(viewOptions);
-                    view.render();
-                }
-            });
+<script>
+    require(["jquery", "js/factories/xblock_validation"], function($, XBlockValidationFactory) {
+        XBlockValidationFactory(
+            ${messages},
+            $.parseJSON("${bool(xblock_url)}".toLowerCase()),  // xblock_url will be None or a string
+            $.parseJSON("${bool(is_root)}".toLowerCase()),  // is_root will be None or a boolean
+            $('div.xblock-validation-messages[data-locator="${xblock.location | h}"]')
+        );
+    });
 </script>
 
-
 % if not is_root:
     % if is_reorderable:
     <li class="studio-xblock-wrapper is-draggable" data-locator="${xblock.location | h}" data-course-key="${xblock.location.course_key | h}">