From 9824eadd4359b44d88a853f05eb2a36713db11ec Mon Sep 17 00:00:00 2001 From: nicola Date: Thu, 9 Jan 2020 10:06:08 +0000 Subject: [PATCH 1/4] Move course validation to back-end for efficiency --- frontend/src/core/helpers.js | 31 -------------- .../modules/editor/global/views/editorView.js | 28 ++----------- plugins/output/adapt/importsource.js | 2 +- plugins/output/adapt/importsourcecheck.js | 2 +- .../adapt/{helpers.js => outputHelpers.js} | 40 ++++++++++++++++++- plugins/output/adapt/publish.js | 11 +++++ routes/import/index.js | 2 +- routes/lang/en.json | 3 ++ 8 files changed, 59 insertions(+), 60 deletions(-) rename plugins/output/adapt/{helpers.js => outputHelpers.js} (87%) diff --git a/frontend/src/core/helpers.js b/frontend/src/core/helpers.js index 5c8269b45c..032d33e839 100644 --- a/frontend/src/core/helpers.js +++ b/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 += "
  • " + alerts[i] + "
    • "; - } - 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); diff --git a/frontend/src/modules/editor/global/views/editorView.js b/frontend/src/modules/editor/global/views/editorView.js index 4e5ee2eea1..c000690b0e 100644 --- a/frontend/src/modules/editor/global/views/editorView.js +++ b/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:

    " + error }); - } - next.call(this, error); - }, this)); - }, - previewProject: function(previewWindow, forceRebuild) { if(Origin.editor.isPreviewPending) { return; diff --git a/plugins/output/adapt/importsource.js b/plugins/output/adapt/importsource.js index f40b9bb841..3c04022da3 100644 --- a/plugins/output/adapt/importsource.js +++ b/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"); diff --git a/plugins/output/adapt/importsourcecheck.js b/plugins/output/adapt/importsourcecheck.js index d9bc9bfcdc..abbab42214 100644 --- a/plugins/output/adapt/importsourcecheck.js +++ b/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"); diff --git a/plugins/output/adapt/helpers.js b/plugins/output/adapt/outputHelpers.js similarity index 87% rename from plugins/output/adapt/helpers.js rename to plugins/output/adapt/outputHelpers.js index ba5c3a5573..428e2a93ff 100644 --- a/plugins/output/adapt/helpers.js +++ b/plugins/output/adapt/outputHelpers.js @@ -277,6 +277,43 @@ 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').charAt(0).toUpperCase() + app.polyglot.t('app.course').slice(1); + errors += courseString + ' "' + data.course[0].title + '" ' + app.polyglot.t('app.doesnotcontain') + ' ' + app.polyglot.t('app.page') + 's\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 = ''; + parents.forEach(parent => { + let parentType = app.polyglot.t('app.' + parent._type).charAt(0).toUpperCase() + app.polyglot.t('app.' + parent._type).slice(1); + let childType = app.polyglot.t('app.children'); + if (children[0] && children[0]._type) childType = app.polyglot.t('app.' + children[0]._type) + 's'; + let found = children.find(child => JSON.stringify(child._parentId) === JSON.stringify(parent._id)); + + if (typeof found === 'undefined') { + errors += parentType + ' "' + parent.title + '" ' + app.polyglot.t('app.doesnotcontain') + ' ' + childType + '\n'; + } + }); + return errors; +} + function ImportError(message, httpStatus) { this.message = message || "Course import failed"; this.httpStatus = httpStatus || 500; @@ -322,5 +359,6 @@ exports = module.exports = { ImportError: ImportError, PartialImportError: PartialImportError, sortContentObjects: sortContentObjects, - cleanUpImport: cleanUpImport + cleanUpImport: cleanUpImport, + validateCourse: validateCourse }; diff --git a/plugins/output/adapt/publish.js b/plugins/output/adapt/publish.js index 080e30da5b..09527dedb7 100644 --- a/plugins/output/adapt/publish.js +++ b/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) { @@ -58,6 +59,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); diff --git a/routes/import/index.js b/routes/import/index.js index da22eb137e..884fc140ce 100644 --- a/routes/import/index.js +++ b/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\/?.*$/); diff --git a/routes/lang/en.json b/routes/lang/en.json index 0fb8d29ad4..3aa9c790f0 100644 --- a/routes/lang/en.json +++ b/routes/lang/en.json @@ -171,6 +171,8 @@ "app.preview": "Preview course", "app.previewing": "Previewing...", "app.forcerebuild": "Force rebuild", + "app.doesnotcontain": "does not contain any", + "app.children": "children", "app.editprofiletitle": "User profile", "app.editprofileinformation": "You can view and modify your user information below.", "app.firstname": "First name", @@ -392,6 +394,7 @@ "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", From 741734d650a555cf4f057cf3e803f0ae3923caf8 Mon Sep 17 00:00:00 2001 From: nicola Date: Fri, 10 Jan 2020 09:45:38 +0000 Subject: [PATCH 2/4] Improve pluralisation --- plugins/output/adapt/outputHelpers.js | 18 +++++++++++++----- routes/lang/en.json | 10 +++++----- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/plugins/output/adapt/outputHelpers.js b/plugins/output/adapt/outputHelpers.js index 428e2a93ff..cff1e2c38f 100644 --- a/plugins/output/adapt/outputHelpers.js +++ b/plugins/output/adapt/outputHelpers.js @@ -285,8 +285,12 @@ function validateCourse(data, cb) { let components = data.component; if (typeof contentObjects === 'undefined') { - let courseString = app.polyglot.t('app.course').charAt(0).toUpperCase() + app.polyglot.t('app.course').slice(1); - errors += courseString + ' "' + data.course[0].title + '" ' + app.polyglot.t('app.doesnotcontain') + ' ' + app.polyglot.t('app.page') + 's\n'; + 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); } @@ -302,13 +306,17 @@ function validateCourse(data, cb) { function iterateThroughChildren(parents, children) { let errors = ''; parents.forEach(parent => { - let parentType = app.polyglot.t('app.' + parent._type).charAt(0).toUpperCase() + app.polyglot.t('app.' + parent._type).slice(1); + let parentType = app.polyglot.t('app.' + parent._type, 1); let childType = app.polyglot.t('app.children'); - if (children[0] && children[0]._type) childType = app.polyglot.t('app.' + children[0]._type) + 's'; + 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') { - errors += parentType + ' "' + parent.title + '" ' + app.polyglot.t('app.doesnotcontain') + ' ' + childType + '\n'; + errors += app.polyglot.t('app.doesnotcontain', { + type: parentType[0].toUpperCase() + parentType.slice(1), + title: parent.title, + childType: childType + }) + '\n'; } }); return errors; diff --git a/routes/lang/en.json b/routes/lang/en.json index 3aa9c790f0..9352beb305 100644 --- a/routes/lang/en.json +++ b/routes/lang/en.json @@ -171,7 +171,7 @@ "app.preview": "Preview course", "app.previewing": "Previewing...", "app.forcerebuild": "Force rebuild", - "app.doesnotcontain": "does not contain any", + "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.", @@ -396,10 +396,10 @@ "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", From 79c7041aa65357115bac68bcf852cc94cba3623a Mon Sep 17 00:00:00 2001 From: nicola Date: Mon, 13 Jan 2020 15:46:05 +0000 Subject: [PATCH 3/4] Handle undefined parents/children --- plugins/output/adapt/outputHelpers.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/plugins/output/adapt/outputHelpers.js b/plugins/output/adapt/outputHelpers.js index cff1e2c38f..143c0b84ef 100644 --- a/plugins/output/adapt/outputHelpers.js +++ b/plugins/output/adapt/outputHelpers.js @@ -305,9 +305,21 @@ function validateCourse(data, cb) { function iterateThroughChildren(parents, children) { let errors = ''; + if (typeof parents === 'undefined') return errors; + parents.forEach(parent => { let parentType = app.polyglot.t('app.' + parent._type, 1); let childType = app.polyglot.t('app.children'); + + if (typeof children === 'undefined') { + errors += app.polyglot.t('app.doesnotcontain', { + type: parentType[0].toUpperCase() + parentType.slice(1), + title: parent.title, + childType: childType + }) + '\n'; + 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)); From c94c8f267d0a4f88851b2d6f028af045fbbf34f5 Mon Sep 17 00:00:00 2001 From: nicola Date: Fri, 17 Jan 2020 16:14:24 +0000 Subject: [PATCH 4/4] Store function in variable --- plugins/output/adapt/outputHelpers.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/plugins/output/adapt/outputHelpers.js b/plugins/output/adapt/outputHelpers.js index 143c0b84ef..3300c93fd0 100644 --- a/plugins/output/adapt/outputHelpers.js +++ b/plugins/output/adapt/outputHelpers.js @@ -307,16 +307,20 @@ 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') { - errors += app.polyglot.t('app.doesnotcontain', { - type: parentType[0].toUpperCase() + parentType.slice(1), - title: parent.title, - childType: childType - }) + '\n'; + appendError(parentType, parent.title, childType); return; } @@ -324,11 +328,7 @@ function iterateThroughChildren(parents, children) { let found = children.find(child => JSON.stringify(child._parentId) === JSON.stringify(parent._id)); if (typeof found === 'undefined') { - errors += app.polyglot.t('app.doesnotcontain', { - type: parentType[0].toUpperCase() + parentType.slice(1), - title: parent.title, - childType: childType - }) + '\n'; + appendError(parentType, parent.title, childType); } }); return errors;