From da35171f9f296f14259b473e1708dc7e3ae8fa84 Mon Sep 17 00:00:00 2001 From: danielghost Date: Fri, 17 Jul 2020 17:41:58 +0100 Subject: [PATCH] Amends to resolve the execution order of statements when the language is changed. Fixes issues with attempting to send statements for models which have been reloaded since the language was changed and Adapt reloaded. Also added an amend to save the duration on visibilityChange to help limit any data loss should terminated data not be sent when closing the course. --- js/adapt-xapi.js | 9 ++-- js/stateModel.js | 45 +++++++++++------- js/statementModel.js | 109 ++++++++++++++++++++++++++++++++----------- 3 files changed, 115 insertions(+), 48 deletions(-) diff --git a/js/adapt-xapi.js b/js/adapt-xapi.js index e23aa3a..60fccb3 100644 --- a/js/adapt-xapi.js +++ b/js/adapt-xapi.js @@ -134,13 +134,14 @@ define([ var languageConfig = Adapt.config.get('_languagePicker'); if (languageConfig && languageConfig._isEnabled && this._restoredLanguage !== lang && this._currentLanguage !== lang) { - // @todo: only send when via a user selection? If `"_showOnCourseLoad": false`, this will still be triggered - Adapt.trigger('xapi:languageChanged', lang); + // only reset if language has changed since the course was started - not neccessary before + var resetState = Adapt.get('_isStarted') && !languageConfig._restoreStateOnLanguageChange; - // only trigger reset if language has changed since the course was started - not neccessary before - if (Adapt.get('_isStarted') && !languageConfig._restoreStateOnLanguageChange) Adapt.trigger('xapi:languageChangedStateReset'); + // @todo: only send when via a user selection? If `"_showOnCourseLoad": false`, this will still be triggered + Adapt.trigger('xapi:languageChanged', lang, resetState); } + this._restoredLanguage = null; this._currentLanguage = lang; }, diff --git a/js/stateModel.js b/js/stateModel.js index 6768a36..bedcf50 100644 --- a/js/stateModel.js +++ b/js/stateModel.js @@ -22,14 +22,15 @@ define([ _storeQuestionResponses: true }, + xAPIWrapper: null, + _isInitialized: false, _isLoaded: false, _isRestored: false, initialize: function(attributes, options) { - this.listenToOnce(Adapt, 'adapt:initialize', this.onAdaptInitialize); - this.listenTo(Adapt, { - 'xapi:languageChangedStateReset': this.onLanguageChangedStateReset, + 'adapt:initialize': this.onAdaptInitialize, + 'xapi:languageChanged': this.onLanguageChanged, 'xapi:stateReset': this.onStateReset }); @@ -54,11 +55,6 @@ define([ }, setupListeners: function() { - this.listenTo(Adapt, { - // ideally core would trigger this event for each model so we don't have to return early for non-component types - 'state:change': this.onTrackableStateChange - }); - this.listenTo(Adapt.course, { 'change:_totalDuration': this.onDurationChange }); @@ -66,6 +62,14 @@ define([ this.listenTo(Adapt.contentObjects, { 'change:_totalDuration': this.onDurationChange }); + + // don't create new listeners for those which are still valid from initial course load + if (this._isInitialized) return; + + this.listenTo(Adapt, { + // ideally core would trigger this event for each model so we don't have to return early for non-component types + 'state:change': this.onTrackableStateChange + }); }, showErrorNotification: function() { @@ -116,7 +120,7 @@ define([ Adapt.trigger('xapi:stateLoaded'); - scope.listenToOnce(Adapt, 'app:dataReady', scope.onDataReady); + scope.listenTo(Adapt, 'app:dataReady', scope.onDataReady); } }); } @@ -126,6 +130,8 @@ define([ var states = this._getStates(); var scope = this; + this._isRestored = false; + Adapt.wait.begin(); Async.each(states, function(id, callback) { @@ -296,10 +302,14 @@ define([ onAdaptInitialize: function() { this.setupListeners(); + + this._isInitialized = true; }, onDurationChange: function(model) { - this._setDurationsData(model); + // don't save durations unless data has been restored - ignore any durations being set via experienced statements on models via `onLanguageChange` listeners + // @todo: remove and re-apply all listeners to (including those in `initialize`) to prevent the need to use the `_isRestored` condition? + if (this._isRestored) this._setDurationsData(model); }, onTrackableStateChange: function(model, state) { @@ -313,12 +323,16 @@ define([ this.reset(); }, - // @todo: resetting could go against cmi5 spec, if course was previosuly completed - can't send multiple "cmi.defined" statements for some verbs - onLanguageChangedStateReset: function() { + // @todo: resetting could go against cmi5 spec, if course was previously completed - can't send multiple "cmi.defined" statements for some verbs + onLanguageChanged: function(lang, isStateReset) { + if (!isStateReset) return; + + this._isRestored = false; + var states = this._getStates(); var statesToReset = states.filter(function(id) { - return id !== DURATIONS_KEY && id !== 'lang'; + return id !== 'lang'; }); var scope = this; @@ -328,12 +342,11 @@ define([ Async.each(statesToReset, function(id, callback) { scope.delete(id, callback); }, function(err) { - if (err) { - scope.showErrorNotification(); - } + if (err) scope.showErrorNotification(); var data = {}; data[COMPONENTS_KEY] = []; + data[DURATIONS_KEY] = []; scope.set(data, { silent: true }); Adapt.wait.end(); diff --git a/js/statementModel.js b/js/statementModel.js index 66aaf2c..788acc9 100644 --- a/js/statementModel.js +++ b/js/statementModel.js @@ -18,24 +18,23 @@ define([ var StatementModel = Backbone.Model.extend({ - xAPIWrapper: null, - _tracking: { _questionInteractions: true, _assessmentsCompletion: false, _assessmentCompletion: true }, + xAPIWrapper: null, + _isInitialized: false, _hasLanguageChanged: false, + _courseSessionStartTime: null, + _currentPageModel: null, _terminate: false, initialize: function(attributes, options) { - this.listenToOnce(Adapt, { - 'adapt:initialize': this.onAdaptInitialize - }); - this.listenTo(Adapt, { - 'xapi:languageChanged': this.onLanguageChanged + 'adapt:initialize': this.onAdaptInitialize, + 'xapi:languageChanged': this.onLanguageChanged, }); this.xAPIWrapper = options.wrapper; @@ -50,9 +49,6 @@ define([ }, setupListeners: function() { - this._onWindowUnload = _.bind(this.onWindowUnload, this); - $(window).on('beforeunload unload', this._onWindowUnload); - this.listenTo(Adapt.contentObjects, { 'change:_isComplete': this.onContentObjectComplete }); @@ -61,6 +57,15 @@ define([ 'change:_isComplete': this.onComponentComplete }); + // don't create new listeners for those which are still valid from initial course load + if (this._isInitialized) return; + + this._onVisibilityChange = _.bind(this.onVisibilityChange, this); + $(document).on('visibilitychange', this._onVisibilityChange); + + this._onWindowUnload = _.bind(this.onWindowUnload, this); + $(window).on('beforeunload unload', this._onWindowUnload); + this.listenTo(Adapt, { 'pageView:ready': this.onPageViewReady, 'router:location': this.onRouterLocation, @@ -139,6 +144,8 @@ define([ var statement = statementModel.getData(model); this.send(statement); + + model.unset('_sessionStartTime', { silent: true }); }, sendQuestionAnswered: function(model) { @@ -206,7 +213,7 @@ define([ if (this._terminate) { this.xAPIWrapper.sendStatement(statement); } else { - this.xAPIWrapper.sendStatement(statement, function(request, obj) { + this.xAPIWrapper.sendStatement(statement, _.bind(function(request, obj) { Adapt.log.debug("[" + obj.id + "]: " + request.status + " - " + request.statusText); switch (request.status) { @@ -223,12 +230,26 @@ define([ this.showErrorNotification(); break; } - }); + }, this)); } }, + setModelSessionStartTime: function(model) { + var time = new Date().getTime(); + + model.set('_sessionStartTime', time); + + // capture start time for course session as models are reloaded on a language change + if (model.get('_type') === "course") this._courseSessionStartTime = time; + }, + setModelDuration: function(model) { - var sessionDuration = new Date().getTime() - model.get('_sessionStartTime'); + var sessionStartTime = model.get('_sessionStartTime'); + + // use stored session start time for reloaded course model following a language change + if (!(model.has('_sessionStartTime') && model.get('_type') === "course")) sessionStartTime = this._courseSessionStartTime; + + var sessionDuration = new Date().getTime() - sessionStartTime; var totalDuration = (model.get('_totalDuration') || 0) + sessionDuration; model.set({ @@ -237,49 +258,72 @@ define([ }); }, - onLanguageChanged: function(lang) { + onLanguageChanged: function(lang, isStateReset) { this._hasLanguageChanged = true; - // send statement if language has changed since the course was started - call in `onAdaptInitialize` is only used initially to ensure correct execution order of statements - if (Adapt.get('_isStarted')) this.sendPreferredLanguage(); + if (Adapt.get('_isStarted')) { + if (this._currentPageModel) { + // send experienced statement to ensure statement is sent before preferred language + this.sendExperienced(this._currentPageModel); + + // reset to bypass call in `onRouterLocation` so experienced statement is not sent + this._currentPageModel = null; + + //this.setModelDuration(Adapt.course); + } + + // send statement if language has changed since the course was started - call in `onAdaptInitialize` is only used initially to ensure correct execution order of statements + this.sendPreferredLanguage(); + } this.set('lang', lang); + + // reset course session start time if the state has been reset + if (isStateReset) this.setModelSessionStartTime(Adapt.course); }, onAdaptInitialize: function() { - this.sendInitialized(); + if (!this._isInitialized) { + this.setModelSessionStartTime(Adapt.course); + + this.sendInitialized(); - Adapt.course.set('_sessionStartTime', new Date().getTime()); + // only called on initial launch if the course contains a language picker - call in `onLanguageChanged` is used for subsequent changes within the current browser session + if (this._hasLanguageChanged) { + this.sendPreferredLanguage(); + + this._hasLanguageChanged = false; + } + } this.setupListeners(); - // only called if course contains a language picker - if (this._hasLanguageChanged) this.sendPreferredLanguage(); + this._isInitialized = true; }, - // add into core? onPageViewReady: function(view) { var model = view.model; - model.set('_sessionStartTime', new Date().getTime()); + // store model so we have a reference to existing model following a language change + this._currentPageModel = model; + + this.setModelSessionStartTime(model); }, onRouterLocation: function() { var previousId = Adapt.location._previousId; - if (!previousId) return; + // bypass if no page model or no previous location + if (!this._currentPageModel || !previousId) return; var model = Adapt.findById(previousId); - if (model.get('_type') === "page") { + if (model && model.get('_type') === "page") { // only record experienced statements for pages this.sendExperienced(model); - - model.unset('_sessionStartTime', { silent: true }); } - // set course duration to ensure State loss is minimised for durations data, if terminate didn't fire - this.setModelDuration(Adapt.course); + this._currentPageModel = null; }, onContentObjectComplete: function(model) { @@ -336,6 +380,15 @@ define([ this.sendResourceExperienced(model); }, + onVisibilityChange: function() { + // set durations to ensure State loss is minimised for durations data, if terminate didn't fire + if (document.visibilityState === "hidden" && !this._terminate) { + if (this._currentPageModel) this.setModelDuration(this._currentPageModel); + + this.setModelDuration(Adapt.course); + } + }, + onWindowUnload: function() { $(window).off('beforeunload unload', this._onWindowUnload);