Add support for OfflineStorage/Bookmarking

[brian-learningpool]

An implementation of offline storage will be required for the xAPI extension in order to allow bookmarking to be compatible.

I think that only a single handler for Adapt.offlineStorage can be registered though. In the case where Spoor and xAPI are both enabled on a course -- which is possible in theory -- we'll probably need some flag to define which is responsible for state, i.e. which gets to manage Adapt.offlineStorage.

I'm interested in your thoughts on this, @oliverfoster?

[oliverfoster]

I think we have to assume a single handler will pick up the offlineStorage get interface but we may have multiple extensions listening for the sets. I think perhaps the backside of the setter can become a synchronous event:

Adapt.trigger("offlineStorage:change", name, value);

How we stop spoor from handling the sets is by binary switch or extension nomination (by name) or something similar I'd imagine?
Any better ideas?

[oliverfoster]

@brian-learningpool As discussed today we really need to have a matrix of xapi vs spoor and when each would need to provide the get interface data for offlineStorage and when each would need to take from the set interface.

We may have situations where the spoor extension might need to provide the learnerId but xapi will need to provide the rest of the data, for example extended text input fields might need to use the offlineStorage.set to store their relevant datas.

At the moment, assessment, bookmarking, accessibility, language switcher and spoor use offlineStorage.set to store their states.

[brian-learningpool]

Hi @oliverfoster, apologies, I'm only getting back to this now after some time away from it.

Does it make sense for multiple extensions to listen for the sets but only one to handle the gets? It could get messy having multiple 'stores' handle different pieces of data. (Wouldn't this require storing the plugin with the key also, so we know which plugin to get the value from again?)

In the develop branch of the xAPI extension right now I have a shouldTrackState property. Spoor doesn't have such a property and always assumes responsibility for maintaining the state. It's possibly safe to proceed along this assumption, as this ticket is really covering an edge case where there are two plugins enabled and capable of managing state.

From my side, I think it's acceptable to give Spoor -- i.e. the first offlineStorage handler implemented -- precedence in this instance, and displaying a console warning to the user about the potential mis-configuration, i.e. that shouldTrackState: true is not a valid configuration on xAPI because of the Spoor extension present and enabled on the course. At least for the first pass, it's probably lowest impact/risk on the existing code so close to the release of v3 that this is handled in the xAPI extension.

In terms of the properties xAPI can store pretty much handle all of the ones currently handled by Spoor, which is enough for the list of plugins you've mentioned above.

[moloko]

Seeing as it's spoor that contains the logic for registering the offlineStorage handler it would be straightforward to update it at some point in the future with a dontTrackState setting to allow an author to stop it from doing that when both xAPI and spoor are enabled and you want xAPI to handle the state tracking.

The most we might consider doing to the core code right now is add in an alert if two different plugins attempt to register offlineStorage handlers

[brian-learningpool]

@moloko / @oliverfoster, I've been experimenting with implementing an offlineStorage handler for xAPI, dependent on the shouldTrackState config, and I think there's a possible issue with this line:
https://github.com/adaptlearning/adapt_framework/blob/master/src/core/js/offlineStorage.js#L14

Assuming offlineStorage is ready all the time, while testing specifically with the Language Picker, doesn't allow the lang and a11y values to be retrieved. Ideally the logic should only default this to true if xAPI is enabled but not tracking state, and I guess also if Spoor is enabled but not tracking state.

Do you have any thoughts?

[brian-learningpool]

I'm not sure how best to conditionally set the offlineStorage ready default when the new Adapt.build object isn't available until after Adapt.offlineStorage has already been instantiated during bootstrapping. I could add something like this to the bottom of offlineStorage.js:

Adapt.on('buildModel:dataLoaded', function() {
  Adapt.offlineStorage = new OfflineStorage();

  return Adapt.offlineStorage;
});

And then I could check if spoor or xapi were in the includes array.

[oliverfoster]

If each extension has a switch which causes it not to register itself, there should no need for core to do conflict management? Don't we just need the core to wait for the plugins to be ready?

To get it to wait we can use the Adapt.wait.queue API at these lines after the dataLoaded event.

Then spoor and xapi can use Adapt.wait.for API upon loading, somewhere around these lines to halt Adapt's execution, until satisfied.

For hand built courses we don't populate the includes array, my understanding of it is that it's a build directive only?

[brian-learningpool]

@moloko, I'm closing this. The core code affected was already merged in 4863684.