Merge pull request #2489 from CanStudios/issue/2487

Move course validation to back-end for efficiency

Author: canstudios-nicolaw

frontend/src/core/helpers.js

@@ -228,37 +228,6 @@ define(function(require){
       return success;
     },
 
-    // checks for at least one child object
-    validateCourseContent: function(currentCourse, callback) {
-      var containsAtLeastOneChild = true;
-      var alerts = [];
-      var iterateOverChildren = function(model, index, doneIterator) {
-        if(!model._childTypes) {
-          return doneIterator();
-        }
-        model.fetchChildren(function(currentChildren) {
-          if (currentChildren.length > 0) {
-            return helpers.forParallelAsync(currentChildren, iterateOverChildren, doneIterator);
-          }
-          containsAtLeastOneChild = false;
-          var children = _.isArray(model._childTypes) ? model._childTypes.join('/') : model._childTypes;
-          alerts.push(model.get('_type') + " '" + model.get('title') + "' missing " + children);
-          doneIterator();
-        });
-      };
-      // start recursion
-      iterateOverChildren(currentCourse, null, function() {
-        var errorMessage = "";
-        if(alerts.length > 0)  {
-          for(var i = 0, len = alerts.length; i < len; i++) {
-            errorMessage += "<li>" + alerts[i] + "</li>";
-          }
-          return callback(new Error(errorMessage));
-        }
-        callback(null, true);
-      });
-    },
-
     isValidEmail: function(value) {
       var regEx = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
       return value.length > 0 && regEx.test(value);

frontend/src/modules/editor/global/views/editorView.js

@@ -44,25 +44,12 @@ define(function(require) {
         'editorView:copy': this.addToClipboard,
         'editorView:copyID': this.copyIdToClipboard,
         'editorView:paste': this.pasteFromClipboard,
-        'editorCommon:download': function() {
-          this.validateProject(function(error) {
-            this.downloadProject();
-          });
-        },
+        'editorCommon:download': this.downloadProject,
         'editorCommon:preview': function(isForceRebuild) {
           var previewWindow = window.open('loading', 'preview');
-          this.validateProject(function(error) {
-            if(error) {
-              return previewWindow.close();
-            }
-            this.previewProject(previewWindow, isForceRebuild);
-          });
+          this.previewProject(previewWindow, isForceRebuild);
         },
-        'editorCommon:export': function() {
-          this.validateProject(function(error) {
-            this.exportProject(error);
-          });
-        }
+        'editorCommon:export': this.exportProject
       });
       this.render();
       this.setupEditor();
@@ -76,15 +63,6 @@ define(function(require) {
       this.renderCurrentEditorView();
     },
 
-    validateProject: function(next) {
-      helpers.validateCourseContent(this.currentCourse, _.bind(function(error) {
-        if(error) {
-          Origin.Notify.alert({ type: 'error', text: "There's something wrong with your course:<br/><br/>" + error });
-        }
-        next.call(this, error);
-      }, this));
-    },
-
     previewProject: function(previewWindow, forceRebuild) {
       if(Origin.editor.isPreviewPending) {
         return;

plugins/output/adapt/importsource.js

@@ -7,7 +7,7 @@ const database = require("../../../lib/database");
 const filestorage = require('../../../lib/filestorage');
 const fs = require("fs-extra");
 const glob = require('glob');
-const helpers = require('./helpers');
+const helpers = require('./outputHelpers');
 const logger = require("../../../lib/logger");
 const mime = require('mime');
 const path = require("path");

plugins/output/adapt/importsourcecheck.js

@@ -6,7 +6,7 @@ const configuration = require('../../../lib/configuration');
 const Constants = require('../../../lib/outputmanager').Constants;
 const database = require("../../../lib/database");
 const fs = require("fs-extra");
-const helpers = require('./helpers');
+const helpers = require('./outputHelpers');
 const IncomingForm = require('formidable').IncomingForm;
 const logger = require("../../../lib/logger");
 const path = require("path");

plugins/output/adapt/helpers.js plugins/output/adapt/outputHelpers.jsplugins/output/adapt/helpers.js renamed to plugins/output/adapt/outputHelpers.js

@@ -277,6 +277,63 @@ function getPluginFrameworkVersionCategory(serverFrameworkVersion, pluginMetaDat
   });
 };
 
+function validateCourse(data, cb) {
+  let errors = '';
+  let contentObjects = data.contentobject;
+  let articles = data.article;
+  let blocks = data.block;
+  let components = data.component;
+
+  if (typeof contentObjects === 'undefined') {
+    let courseString = app.polyglot.t('app.course');
+    errors += app.polyglot.t('app.doesnotcontain', {
+      type: courseString[0].toUpperCase() + courseString.slice(1),
+      title: data.course[0].title,
+      childType: app.polyglot.t('app.page', 0)
+    }) + '\n';
+    return cb(errors, false);
+  }
+
+  errors += iterateThroughChildren(contentObjects, articles);
+  errors += iterateThroughChildren(articles, blocks);
+  errors += iterateThroughChildren(blocks, components);
+
+  if (errors.length !== 0) return cb(errors, false);
+
+  return cb(null, true);
+}
+
+function iterateThroughChildren(parents, children) {
+  let errors = '';
+  if (typeof parents === 'undefined') return errors;
+
+  const appendError = (parentType, parentTitle, childType) => {
+    errors += app.polyglot.t('app.doesnotcontain', {
+      type: parentType[0].toUpperCase() + parentType.slice(1),
+      title: parentTitle,
+      childType: childType
+    }) + '\n';
+  };
+
+  parents.forEach(parent => {
+    let parentType = app.polyglot.t('app.' + parent._type, 1);
+    let childType = app.polyglot.t('app.children');
+
+    if (typeof children === 'undefined') {
+      appendError(parentType, parent.title, childType);
+      return;
+    }
+
+    if (children[0] && children[0]._type) childType = app.polyglot.t('app.' + children[0]._type, 0);
+    let found = children.find(child => JSON.stringify(child._parentId) === JSON.stringify(parent._id));
+
+    if (typeof found === 'undefined') {
+      appendError(parentType, parent.title, childType);
+    }
+  });
+  return errors;
+}
+
 function ImportError(message, httpStatus) {
   this.message = message || "Course import failed";
   this.httpStatus = httpStatus || 500;
@@ -322,5 +379,6 @@ exports = module.exports = {
   ImportError: ImportError,
   PartialImportError: PartialImportError,
   sortContentObjects: sortContentObjects,
-  cleanUpImport: cleanUpImport
+  cleanUpImport: cleanUpImport,
+  validateCourse: validateCourse
 };

plugins/output/adapt/publish.js

@@ -12,6 +12,7 @@ const helpers = require('../../../lib/helpers');
 const installHelpers = require('../../../lib/installHelpers');
 const logger = require('../../../lib/logger');
 const origin = require('../../../');
+const outputHelpers = require('./outputHelpers');
 const usermanager = require('../../../lib/usermanager');
 
 function publishCourse(courseId, mode, request, response, next) {
@@ -59,6 +60,16 @@ function publishCourse(courseId, mode, request, response, next) {
         callback(null);
       });
     },
+    // validate the course data
+    function(callback) {
+      outputHelpers.validateCourse(outputJson, function(error, isValid) {
+        if (error || !isValid) {
+          return callback({ message: error });
+        }
+
+        callback(null);
+      });
+    },
     //
     function(callback) {
       var temporaryThemeFolder = path.join(SRC_FOLDER, Constants.Folders.Theme, customPluginName);

routes/import/index.js

@@ -3,7 +3,7 @@ var permissions = require('../../lib/permissions');
 var server = module.exports = require('express')();
 var usermanager = require('../../lib/usermanager');
 var util = require('util');
-var helpers = require('../../plugins/output/adapt/helpers');
+var helpers = require('../../plugins/output/adapt/outputHelpers');
 
 // stop any auto permissions checks
 permissions.ignoreRoute(/^\/import\/?.*$/);

routes/lang/en.json

@@ -165,6 +165,8 @@
   "app.preview": "Preview course",
   "app.previewing": "Previewing...",
   "app.forcerebuild": "Force rebuild",
+  "app.doesnotcontain": "%{type} '%{title}' does not contain any %{childType}",
+  "app.children": "children",
   "app.editprofiletitle": "User profile",
   "app.editprofileinformation": "You can view and modify your user information below.",
   "app.firstname": "First name",
@@ -386,11 +388,12 @@
   "app.editormenusettings": "Menu picker",
   "app.editorextensions": "Manage extensions",
   "app.editing": "Editing %{type}: %{text}",
+  "app.course": "course",
   "app.menu": "menu",
-  "app.page": "page",
-  "app.article": "article",
-  "app.block": "block",
-  "app.component": "component",
+  "app.page": "page |||| pages",
+  "app.article": "article |||| articles",
+  "app.block": "block |||| blocks",
+  "app.component": "component |||| components",
   "app.addedDefault": "Add to new courses by default?",
   "app.errorloadconfig": "Failed to load configuration settings for %{course}",
   "app.errorloadfiles": "Failed to load content files",