SJRK-268: generalized custom theme tests to work for any custom theme

src/ui/storyTellingServerUI.js

@@ -109,28 +109,16 @@ sjrk.storyTelling.loadBrowse = function (clientConfig, options) {
 };
 
 /* Gets the current theme from the server and loads associated  files if and
- * only if the current theme is not set to the base theme. Once complete, the
- * provided callback function is called and the theme name is passed into it.
- * - "callback": a function to call once everything has completed. Will be called
- *               regardless of whether theme information was specified or retrieved
+ * only if the current theme is not set to the base theme. Returns a promise.
+ * If the promise resolves, it will contain the clientConfig information.
  */
-sjrk.storyTelling.loadThemedPage = function (callback) {
+sjrk.storyTelling.loadTheme = function () {
     var loadPromise = fluid.promise();
 
-    var callbackFunction = typeof callback === "function" ? callback : fluid.getGlobalValue(callback);
-
     $.get("/clientConfig").then(function (data) {
         if (data.theme !== data.baseTheme) {
-            sjrk.storyTelling.loadCustomThemeFiles(callbackFunction, data).then(function () {
-                loadPromise.resolve(data);
-            }, function (jqXHR, textStatus, errorThrown) {
-                loadPromise.reject({
-                    isError: true,
-                    message: errorThrown
-                });
-            });
+            fluid.promise.follow(sjrk.storyTelling.loadCustomThemeFiles(data), loadPromise);
         } else {
-            callbackFunction(data);
             loadPromise.resolve(data);
         }
     }, function (jqXHR, textStatus, errorThrown) {
@@ -144,36 +132,39 @@ sjrk.storyTelling.loadThemedPage = function (callback) {
 };
 
 /* Loads CSS and JavaScript files for the provided theme into the page markup.
- * If JavaScript file loading is successful, the callback function is called.
- * - "callback": a function to call once everything has completed
+ * Returns a promise. If the promise resolves, it will contain the clientConfig.
  * - "clientConfig": a collection of client config values consisting of
  *     - "theme": the current theme of the site
  *     - "baseTheme": the base theme of the site
  *     - "authoringEnabled": indicates whether story saving and editing are enabled
  */
-sjrk.storyTelling.loadCustomThemeFiles = function (callback, clientConfig) {
+sjrk.storyTelling.loadCustomThemeFiles = function (clientConfig) {
     var loadPromise = fluid.promise();
 
-    var cssUrl = fluid.stringTemplate("/css/%theme.css", {theme: clientConfig.theme});
-    var scriptUrl = fluid.stringTemplate("/js/%theme.js", {theme: clientConfig.theme});
+    if (clientConfig.theme !== clientConfig.baseTheme) {
+        var cssUrl = fluid.stringTemplate("/css/%theme.css", {theme: clientConfig.theme});
+        var scriptUrl = fluid.stringTemplate("/js/%theme.js", {theme: clientConfig.theme});
 
-    $("<link/>", {
-        rel: "stylesheet",
-        type: "text/css",
-        href: cssUrl
-    }).appendTo("head");
+        $("<link/>", {
+            rel: "stylesheet",
+            type: "text/css",
+            href: cssUrl
+        }).appendTo("head");
 
-    $.getScript(scriptUrl, function () {
-        if (typeof callback === "function") {
-            var callbackResult = callback(clientConfig);
-            loadPromise.resolve(callbackResult);
-        }
-    }).fail(function (jqXHR, textStatus, errorThrown) {
-        loadPromise.reject({
-            isError: true,
-            message: errorThrown
+        // TODO: This method of loading produces a potential race condition
+        // See SJRK-272: https://issues.fluidproject.org/browse/SJRK-272
+        $.getScript(scriptUrl, function () {
+            loadPromise.resolve(clientConfig);
+        }).fail(function (jqXHR, textStatus, errorThrown) {
+            loadPromise.reject({
+                isError: true,
+                message: errorThrown
+            });
         });
-    });
+    } else {
+        // The theme is the base theme, no custom files need to be loaded
+        loadPromise.resolve(clientConfig);
+    }
 
     return loadPromise;
 };

tests/ui/js/storyTellingServerUITests.js

@@ -69,98 +69,94 @@ https://raw.githubusercontent.com/fluid-project/sjrk-story-telling/master/LICENS
         });
     });
 
-    var loadThemedPageTestCases = {
-        base: {
-            theme: "base",
-            baseTheme: "base",
-            authoringEnabled: true
-        },
-        learningReflections: {
-            theme: "learningReflections",
-            baseTheme: "base",
-            authoringEnabled: true
-        }
-    };
-
     fluid.defaults("sjrk.storyTelling.storyTellingServerUiTester", {
-        gradeNames: ["fluid.test.testCaseHolder"],
+        gradeNames: ["fluid.modelComponent", "fluid.test.testCaseHolder"],
+        baseTestCase: {
+            clientConfig: {
+                theme: "base",
+                baseTheme: "base",
+                authoringEnabled: true
+            }
+        },
+        members: {
+            clientConfig: {
+                theme: "customTheme",
+                baseTheme: "base",
+                authoringEnabled: true
+            }
+        },
         modules: [{
             name: "Test Storytelling Server UI code",
             tests: [{
-                name: "Test themed page loading functions",
-                expect: 6,
+                name: "Test themed page loading functions with mock values",
+                expect: 1,
                 sequence: [{
-                    // call the load themed page function, forcing the base theme response
-                    task: "sjrk.storyTelling.storyTellingServerUiTester.loadThemedPageSingleTest",
-                    args: ["sjrk.storyTelling.testUtils.callbackVerificationFunction", loadThemedPageTestCases.base.theme],
-                    resolve: "jqUnit.assertDeepEq",
-                    resolveArgs: ["The themed page load resolved as expected", loadThemedPageTestCases.base, "{arguments}.0"]
+                    funcName: "sjrk.storyTelling.storyTellingServerUiTester.setupMockServer",
+                    args: ["/clientConfig", "{that}.options.baseTestCase.clientConfig", "application/json"]
                 },{
-                    // call the load themed page function, forcing the Learning Reflections theme response
-                    task: "sjrk.storyTelling.storyTellingServerUiTester.loadThemedPageSingleTest",
-                    args: ["sjrk.storyTelling.testUtils.callbackVerificationFunction", loadThemedPageTestCases.learningReflections.theme],
+                    task: "sjrk.storyTelling.loadTheme",
                     resolve: "jqUnit.assertDeepEq",
-                    resolveArgs: ["The themed page load resolved as expected", loadThemedPageTestCases.learningReflections, "{arguments}.0"]
+                    resolveArgs: ["The themed page load resolved as expected", "{that}.options.baseTestCase.clientConfig", "{arguments}.0"]
                 },{
-                    funcName: "sjrk.storyTelling.storyTellingServerUiTester.assertCustomCssLoaded",
-                    args: ["learningReflections.css", 1]
+                    funcName: "sjrk.storyTelling.storyTellingServerUiTester.teardownMockServer"
+                }]
+            },{
+                name: "Test themed page loading functions with server config values",
+                expect: 3,
+                sequence: [{
+                    task: "$.get",
+                    args: ["/clientConfig"],
+                    resolve: "fluid.set",
+                    resolveArgs: ["{that}", "clientConfig", "{arguments}.0"]
                 },{
-                    // test the CSS/JS injection function directly
-                    funcName: "sjrk.storyTelling.loadCustomThemeFiles",
-                    args: ["sjrk.storyTelling.testUtils.callbackVerificationFunction", {"theme": "learningReflections"}]
+                    task: "sjrk.storyTelling.loadTheme",
+                    resolve: "jqUnit.assertEquals",
+                    resolveArgs: ["The themed page load resolved as expected", "{that}.clientConfig.theme", "{arguments}.0.theme"]
                 },{
-                    funcName: "sjrk.storyTelling.storyTellingServerUiTester.assertCustomCssLoaded",
-                    args: ["learningReflections.css", 2]
+                    funcName: "sjrk.storyTelling.storyTellingServerUiTester.verifyCustomCssLoaded",
+                    args: ["{that}.clientConfig", 1]
+                },{
+                    task: "sjrk.storyTelling.loadCustomThemeFiles",
+                    args: ["{that}.clientConfig"],
+                    resolve: "sjrk.storyTelling.storyTellingServerUiTester.verifyCustomCssLoaded",
+                    resolveArgs: ["{that}.clientConfig", 2]
                 }]
             }]
         }]
     });
 
-    sjrk.storyTelling.storyTellingServerUiTester.loadThemedPageSingleTest = function (callback, expectedTheme) {
-        var loadPromise = fluid.promise();
-
-        sjrk.storyTelling.storyTellingServerUiTester.setupMockServer("/clientConfig", JSON.stringify({
-            theme: expectedTheme,
-            baseTheme: "base",
-            authoringEnabled: true
-        }));
+    sjrk.storyTelling.storyTellingServerUiTester.loadClientConfigFromServer = function (url) {
+        var configPromise = fluid.promise();
 
-        sjrk.storyTelling.loadThemedPage(callback).then(function (clientConfig) {
-            loadPromise.resolve(clientConfig);
-        }, function () {
-            loadPromise.reject();
+        $.get(url).then(function (data) {
+            configPromise.resolve(data);
+        }, function (jqXHR, textStatus, errorThrown) {
+            configPromise.reject({
+                isError: true,
+                message: errorThrown
+            });
         });
 
-        sjrk.storyTelling.storyTellingServerUiTester.teardownMockServer();
-
-        return loadPromise;
+        return configPromise;
     };
 
-    sjrk.storyTelling.storyTellingServerUiTester.setupMockServer = function (url, responseData) {
+    sjrk.storyTelling.storyTellingServerUiTester.setupMockServer = function (url, clientConfig) {
         mockServer = sinon.createFakeServer();
         mockServer.respondImmediately = true;
-
-        mockServer.respondWith(url, [200, { "Content-Type": "application/json" }, responseData]);
+        mockServer.respondWith(url, [200, { "Content-Type": "application/json"}, JSON.stringify(clientConfig)]);
     };
 
     sjrk.storyTelling.storyTellingServerUiTester.teardownMockServer = function () {
         mockServer.restore();
     };
 
-    sjrk.storyTelling.storyTellingServerUiTester.assertCustomCssLoaded = function (expectedFileName, expectedInstanceCount) {
-        var cssFilesLinked = fluid.transform(fluid.getMembers($("link"), "href"), function (fileUrl) {
-            return fileUrl.split("/css/")[1];
-        });
-
-        var actualInstanceCount = 0;
-
-        fluid.each(cssFilesLinked, function (fileName) {
-            if (fileName === expectedFileName) {
-                actualInstanceCount++;
-            }
-        });
+    sjrk.storyTelling.storyTellingServerUiTester.verifyCustomCssLoaded = function (clientConfig, expectedCssInstanceCount) {
+        if (clientConfig.theme === clientConfig.baseTheme) {
+            expectedCssInstanceCount = 0; // if no custom theme is set, we actually expect zero custom files
+        }
 
-        jqUnit.assertEquals("Linked CSS files include the expected custom theme file the expected number of instances", expectedInstanceCount, actualInstanceCount);
+        var actualInstanceCount = $("link[href$=\"" + clientConfig.theme + ".css\"]").length;
+        jqUnit.assertEquals("Custom theme CSS file is linked the expected number of instances", expectedCssInstanceCount, actualInstanceCount);
     };
 
     fluid.defaults("sjrk.storyTelling.storyTellingServerUiTest", {

tests/ui/js/testUtils.js

@@ -170,16 +170,6 @@ https://raw.githubusercontent.com/fluid-project/sjrk-story-telling/master/LICENS
         jqUnit.assertEquals("Number of remaining blocks is expected #: " + expectedNumberOfBlocks, expectedNumberOfBlocks, managedComponentRegistryAsArray.length);
     };
 
-    /* A simple function that asserts that it was called with a truthy value,
-     * intended for use as a callback
-     * - "value": a value (hopefully truthy)
-     */
-    sjrk.storyTelling.testUtils.callbackVerificationFunction = function (value) {
-        if (value) {
-            jqUnit.assert("Callback was successfully called with value: " + JSON.stringify(value));
-        }
-    };
-
     /* Alters URL without pageload, via code from StackOverflow
      * https://stackoverflow.com/questions/10970078/modifying-a-query-string-without-reloading-the-page
      * - "queryString": the query string to append to the current page URL

themes/base/storyBrowse.html

@@ -115,7 +115,7 @@
 
 <script>
     $(document).ready(function () {
-        sjrk.storyTelling.loadThemedPage(function (clientConfig) {
+        sjrk.storyTelling.loadTheme().then(function (clientConfig) {
             sjrk.storyTelling.loadBrowse(clientConfig);
         });
     });

themes/base/storyEdit.html

@@ -128,7 +128,7 @@
 
 <script>
     $(document).ready(function () {
-        sjrk.storyTelling.loadThemedPage(function (clientConfig) {
+        sjrk.storyTelling.loadTheme().then(function (clientConfig) {
             sjrk.storyTelling[clientConfig.theme].page.storyEdit({
                 pageSetup: {
                     authoringEnabled: clientConfig.authoringEnabled

themes/base/storyView.html

@@ -116,7 +116,7 @@
 
 <script>
     $(document).ready(function () {
-        sjrk.storyTelling.loadThemedPage(function (clientConfig) {
+        sjrk.storyTelling.loadTheme().then(function (clientConfig) {
             sjrk.storyTelling.loadStoryFromParameter(clientConfig);
         });
     });

themes/cities/css/cities.css

@@ -95,7 +95,7 @@ h3:empty {
     align-items: center;
     position: relative;
     flex-wrap: wrap;
-    top: 1.75rem;
+    /* top: 1.75rem; */
     justify-content: flex-end;
 }
 

themes/cities/templates/cities-masthead.handlebars

@@ -7,5 +7,5 @@
             <li><a href="storyBrowse.html">{{localizedMessages.link_browse_text}}</a></li>
         </ul>
     </nav>
-    <a class="sjrk-st-edit-link" href="{{localizedMessages.editor_url}}">{{localizedMessages.link_edit_text}}</a>
+    {{!--<a class="sjrk-st-edit-link" href="{{localizedMessages.editor_url}}">{{localizedMessages.link_edit_text}}</a>--}}
 </div>

themes/learningReflections/introduction.html

@@ -94,7 +94,7 @@
 
 <script>
     $(document).ready(function () {
-        sjrk.storyTelling.loadThemedPage(function (clientConfig) {
+        sjrk.storyTelling.loadTheme().then(function (clientConfig) {
             sjrk.storyTelling[clientConfig.theme].page.introduction({});
         });
     });

themes/learningReflections/resources.html

@@ -94,7 +94,7 @@
 
 <script>
     $(document).ready(function () {
-        sjrk.storyTelling.loadThemedPage(function (clientConfig) {
+        sjrk.storyTelling.loadTheme().then(function (clientConfig) {
             sjrk.storyTelling[clientConfig.theme].page.resources({});
         });
     });

themes/learningReflections/workshops.html

@@ -94,7 +94,7 @@
 
 <script>
     $(document).ready(function () {
-        sjrk.storyTelling.loadThemedPage(function (clientConfig) {
+        sjrk.storyTelling.loadTheme().then(function (clientConfig) {
             sjrk.storyTelling[clientConfig.theme].page.workshops({});
         });
     });