From 768eb2a64bd268b74f7c7da2de50347e858efd9b Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Wed, 29 Aug 2018 14:30:31 +0200 Subject: [PATCH 1/3] Remove unkown todo comment --- h5p.classes.php | 1 - 1 file changed, 1 deletion(-) diff --git a/h5p.classes.php b/h5p.classes.php index 6f48f53..35be986 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -3682,7 +3682,6 @@ class H5PContentValidator { if (isset($file->copyright)) { $this->validateGroup($file->copyright, $this->getCopyrightSemantics()); - // TODO: We'll need to do something here about getMetadataSemantics() if we change the widgets } } From e094da76fa545a152f969707b2647e1b0050166d Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Thu, 30 Aug 2018 12:33:24 +0200 Subject: [PATCH 2/3] HFP-1900 Remove metadata/extras from upgrade for main content --- js/h5p-content-upgrade-process.js | 71 +++++++++++-------------------- js/h5p-content-upgrade-worker.js | 7 ++- js/h5p-content-upgrade.js | 25 ++++------- 3 files changed, 36 insertions(+), 67 deletions(-) diff --git a/js/h5p-content-upgrade-process.js b/js/h5p-content-upgrade-process.js index 5277eb6..3562d6d 100644 --- a/js/h5p-content-upgrade-process.js +++ b/js/h5p-content-upgrade-process.js @@ -7,7 +7,7 @@ H5P.ContentUpgradeProcess = (function (Version) { * @class * @namespace H5P */ - function ContentUpgradeProcess(name, oldVersion, newVersion, params, extras, id, loadLibrary, done) { + function ContentUpgradeProcess(name, oldVersion, newVersion, params, id, loadLibrary, done) { var self = this; // Make params possible to work with @@ -24,38 +24,28 @@ H5P.ContentUpgradeProcess = (function (Version) { }); } - // Make extras possible to work with - for (var element in extras) { - if (extras[element] !== undefined) { - try { - extras[element] = JSON.parse(extras[element]); - if (!(extras[element] instanceof Object)) { - throw true; - } - } - catch (event) { - return done({ - type: 'errorExtrasBroken', - id: id - }); - } - } - } - self.loadLibrary = loadLibrary; - self.upgrade(name, oldVersion, newVersion, params, extras, function (err, result) { + self.upgrade(name, oldVersion, newVersion, params, function (err, upgradedParams) { if (err) { return done(err); } - done(null, JSON.stringify(params), JSON.stringify(extras)); + done(null, JSON.stringify(upgradedParams)); }); } /** + * Run content upgrade. * + * @public + * @param {string} name + * @param {Version} oldVersion + * @param {Version} newVersion + * @param {Object} params Only for subcontent + * @param {Function} done Only for subcontent + * @param {Object} [metadata] Only for subcontent */ - ContentUpgradeProcess.prototype.upgrade = function (name, oldVersion, newVersion, params, extras, done) { + ContentUpgradeProcess.prototype.upgrade = function (name, oldVersion, newVersion, params, done, metadata) { var self = this; // Load library details and upgrade routines @@ -65,7 +55,7 @@ H5P.ContentUpgradeProcess = (function (Version) { } // Run upgrade routines on params - self.processParams(library, oldVersion, newVersion, params, extras, function (err, params, extras) { + self.processParams(library, oldVersion, newVersion, params, metadata, function (err, params, metadata) { if (err) { return done(err); } @@ -79,7 +69,7 @@ H5P.ContentUpgradeProcess = (function (Version) { next(err); }); }, function (err) { - done(err, params, extras); + done(err, params, metadata); }); }); }); @@ -95,7 +85,7 @@ H5P.ContentUpgradeProcess = (function (Version) { * @param {Object} params * @param {Function} next */ - ContentUpgradeProcess.prototype.processParams = function (library, oldVersion, newVersion, params, extras, next) { + ContentUpgradeProcess.prototype.processParams = function (library, oldVersion, newVersion, params, metadata, next) { if (H5PUpgrades[library.name] === undefined) { if (library.upgradesScript) { // Upgrades script should be loaded so the upgrades should be here. @@ -130,17 +120,11 @@ H5P.ContentUpgradeProcess = (function (Version) { try { unnecessaryWrapper(params, function (err, upgradedParams, upgradedExtras) { params = upgradedParams; - /** - * "params" (and "extras", e.g. metadata) flow through each update function and are changed - * if necessary. Since "extras" was added later and old update functions don't return - * it, we need to ignore undefined values here -- or change every single update function - * in all content types. - */ - if (upgradedExtras) { - extras = upgradedExtras; + if (upgradedExtras && upgradedExtras.metadata) { // Optional + metadata = upgradedExtras.metadata; } nextMinor(err); - }, extras); + }, {metadata: metadata}); } catch (err) { if (console && console.log) { @@ -154,7 +138,7 @@ H5P.ContentUpgradeProcess = (function (Version) { }, nextMajor); } }, function (err) { - next(err, params, extras); + next(err, params, metadata); }); }; @@ -195,24 +179,17 @@ H5P.ContentUpgradeProcess = (function (Version) { return done(); // Larger or same version that's available } - // Currently, we only use metadata as additional things that might need change - var extras = { - metadata: params.metadata - }; - // A newer version is available, upgrade params - return self.upgrade(availableLib[0], usedVer, availableVer, params.params, extras, function (err, upgraded, extras) { + return self.upgrade(availableLib[0], usedVer, availableVer, params.params, function (err, upgradedParams, upgradedMetadata) { if (!err) { params.library = availableLib[0] + ' ' + availableVer.major + '.' + availableVer.minor; - params.params = upgraded; - if (extras) { - if (extras.metadata) { - params.metadata = extras.metadata; - } + params.params = upgradedParams; + if (upgradedMetadata) { + params.metadata = upgradedMetadata; } } done(err, params); - }); + }, params.metadata); } } done(); diff --git a/js/h5p-content-upgrade-worker.js b/js/h5p-content-upgrade-worker.js index af0be94..26ad038 100644 --- a/js/h5p-content-upgrade-worker.js +++ b/js/h5p-content-upgrade-worker.js @@ -9,7 +9,7 @@ var libraryLoadedCallback; var messageHandlers = { newJob: function (job) { // Start new job - new H5P.ContentUpgradeProcess(job.name, new H5P.Version(job.oldVersion), new H5P.Version(job.newVersion), job.params, job.extras, job.id, function loadLibrary(name, version, next) { + new H5P.ContentUpgradeProcess(job.name, new H5P.Version(job.oldVersion), new H5P.Version(job.newVersion), job.params, job.id, function loadLibrary(name, version, next) { // TODO: Cache? postMessage({ action: 'loadLibrary', @@ -17,7 +17,7 @@ var messageHandlers = { version: version.toString() }); libraryLoadedCallback = next; - }, function done(err, result, extras) { + }, function done(err, result) { if (err) { // Return error postMessage({ @@ -33,8 +33,7 @@ var messageHandlers = { postMessage({ action: 'done', id: job.id, - params: result, - extras: extras + params: result }); }); }, diff --git a/js/h5p-content-upgrade.js b/js/h5p-content-upgrade.js index de17fc1..9c1e4ce 100644 --- a/js/h5p-content-upgrade.js +++ b/js/h5p-content-upgrade.js @@ -116,7 +116,7 @@ // Register message handlers var messageHandlers = { done: function (result) { - self.workDone(result.id, result.params, result.extras, this); + self.workDone(result.id, result.params, this); }, error: function (error) { self.printError(error.err); @@ -184,7 +184,7 @@ self.token = inData.token; // Start processing - self.processBatch(inData.params, {metadata: inData.metadata}); + self.processBatch(inData.params); }); }; @@ -202,7 +202,7 @@ * * @param {Object} parameters */ - ContentUpgrade.prototype.processBatch = function (parameters, extras) { + ContentUpgrade.prototype.processBatch = function (parameters) { var self = this; // Track upgraded params @@ -210,7 +210,6 @@ // Track current batch self.parameters = parameters; - self.extras = extras; // Create id mapping self.ids = []; @@ -248,10 +247,6 @@ self.current++; self.working++; - var extras = { - metadata: (self.extras.metadata && self.extras.metadata[id]) ? self.extras.metadata[id] : undefined - }; - if (worker) { worker.postMessage({ action: 'newJob', @@ -259,12 +254,11 @@ name: info.library.name, oldVersion: info.library.version, newVersion: self.version.toString(), - params: self.parameters[id], - extras: extras + params: self.parameters[id] }); } else { - new H5P.ContentUpgradeProcess(info.library.name, new Version(info.library.version), self.version, self.parameters[id], extras, id, function loadLibrary(name, version, next) { + new H5P.ContentUpgradeProcess(info.library.name, new Version(info.library.version), self.version, self.parameters[id], id, function loadLibrary(name, version, next) { self.loadLibrary(name, version, function (err, library) { if (library.upgradesScript) { self.loadScript(library.upgradesScript, function (err) { @@ -279,13 +273,13 @@ } }); - }, function done(err, result, extras) { + }, function done(err, result) { if (err) { self.printError(err); return ; } - self.workDone(id, result, extras); + self.workDone(id, result); }); } }; @@ -293,7 +287,7 @@ /** * */ - ContentUpgrade.prototype.workDone = function (id, result, extras, worker) { + ContentUpgrade.prototype.workDone = function (id, result, worker) { var self = this; self.working--; @@ -308,8 +302,7 @@ self.nextBatch({ libraryId: self.version.libraryId, token: self.token, - params: JSON.stringify(self.upgraded), - extras: extras + params: JSON.stringify(self.upgraded) }); } }; From e74fb6009ac980f4bb8f768865784cba33e22e7a Mon Sep 17 00:00:00 2001 From: Paal Joergensen Date: Thu, 30 Aug 2018 12:35:46 +0200 Subject: [PATCH 3/3] Use title from metadata object when creating xAPI stmt --- js/h5p-x-api-event.js | 6 +++--- js/h5p.js | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/js/h5p-x-api-event.js b/js/h5p-x-api-event.js index c1d6c66..4cec937 100644 --- a/js/h5p-x-api-event.js +++ b/js/h5p-x-api-event.js @@ -133,9 +133,10 @@ H5P.XAPIEvent.prototype.setObject = function (instance) { } } else { - if (H5PIntegration && H5PIntegration.contents && H5PIntegration.contents['cid-' + instance.contentId].title) { + var content = H5P.getContentForInstance(instance.contentId); + if (content && content.metadata && content.metadata.title) { this.data.statement.object.definition.name = { - "en-US": H5P.createTitle(H5PIntegration.contents['cid-' + instance.contentId].title) + "en-US": H5P.createTitle(content.metadata.title) }; } } @@ -150,7 +151,6 @@ H5P.XAPIEvent.prototype.setObject = function (instance) { */ H5P.XAPIEvent.prototype.setContext = function (instance) { if (instance.parent && (instance.parent.contentId || instance.parent.subContentId)) { - var parentId = instance.parent.subContentId === undefined ? instance.parent.contentId : instance.parent.subContentId; this.data.statement.context = { "contextActivities": { "parent": [ diff --git a/js/h5p.js b/js/h5p.js index 38f6022..4330ade 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -2140,6 +2140,20 @@ H5P.createTitle = function (rawTitle, maxLength) { contentUserDataAjax(contentId, dataId, subContentId, undefined, null); }; + /** + * Function for getting content for a certain ID + * + * @param {number} contentId + * @return {Object} + */ + H5P.getContentForInstance = function (contentId) { + var key = 'cid-' + contentId; + var exists = H5PIntegration && H5PIntegration.contents && + H5PIntegration.contents[key]; + + return exists ? H5PIntegration.contents[key] : undefined; + }; + /** * Prepares the content parameters for storing in the clipboard. *