TinCanJS icon indicating copy to clipboard operation
TinCanJS copied to clipboard

How to load activity_id from query string instead of config

Open deepinder19 opened this issue 5 years ago • 3 comments

_initFromQueryString in "TinCanJS/build/tincan-node.js" loads activity_id from url and creates a new Tincan.activity object. But if activity details are found in config, new activity object is created using those details and it overrides the activity object previously created using the url parameters.

Is there a way to let the url parameters set the activity object, or is it intended to be overridden by config?

deepinder19 avatar Apr 02 '19 19:04 deepinder19

It makes sense to me that the we should wait to call _initFromQueryString until after we've prepopulated the activity with the rest of the contents of cfg.

We could just move this:

https://github.com/RusticiSoftware/TinCanJS/blob/8733f14ddcaeea77a0579505300bc8f38921a6b1/src/TinCan.js#L152-L154

down below this:

https://github.com/RusticiSoftware/TinCanJS/blob/8733f14ddcaeea77a0579505300bc8f38921a6b1/src/TinCan.js#L185-L187

@brianjmiller, you probably understand more of the context on this project than I do. Do you have any problem with this? Was the intention here to let the config overwrite the URL parameters?

reedmclean avatar Apr 03 '19 19:04 reedmclean

@brianjmiller with current implementation I am not able to overwrite "activity" using url parameters but looking at the code its looks like it was intended to have the ability to overwrite activity from the url. This looks like a bug to me.

It looks like a simple change and I'd be happy to make a pull request if you can confirm. Thanks :slightly_smiling_face:

deepinder19 avatar Apr 08 '19 20:04 deepinder19

I guess this comes down to a question of who gets to decide what the activity id is, the content author or the launching system.

The strongest argument I can see in favor of having the launch parameter overwrite the config is that cmi5 comes down in favor of the launching system deciding the activity id:

When the Object is the AU, the value of the Object's "id" property for a given AU MUST match the activityId defined in the launch URL

https://github.com/AICC/CMI-5_Spec_Current/blob/quartz/cmi5_spec.md#94-object

But on the other hand, prioritizing the config value is the current behavior, meaning this could be a breaking change for anybody relying on that behavior. I don't know if we have any way of identifying if anybody is doing this.

A middle route would be to keep the current behavior, but add an additional config setting that would allow the launch parameter to take priority if provided, and fall back to the config value if not.

Probably it's cleanest to follow cmi5 and update the version number of TinCanJS.

garemoko avatar Apr 09 '19 08:04 garemoko