adaptlearning / adapt-contrib-spoor

A basic scorm tracking plugin for Adapt
GNU General Public License v3.0
14 stars 42 forks source link

Attempt to getInteractionCount on LMS that doesn't support this SCORM 1.2 feature #294

Closed paulstevendev closed 11 months ago

paulstevendev commented 11 months ago

I am seeing an issue on an LMS where an error dialogue is displaying after sumitting a question.

image

Looking at the code I can see it is calling this.scorm.getInteractionCount() inside the onQuestionRecordInteraction function within adapt-stateful-session.js

So it would appear this code does not have any check to see if the LMS supports cmi.interaction._count

I am getting the error every time I answer a question. It is possibly an issue with this particular LMS but it is the only LMS we use that does not have this optional SCORM 1.2 element. So it would be good to hear if anyone else has tested this on an LMS that does not support cmi.interactions._count

https://github.com/adaptlearning/adapt-contrib-spoor/blob/f07a6032a687b6653c4dbcd903797663fa3ec665/js/adapt-stateful-session.js#L205C2-L220C4

oliverfoster commented 11 months ago

The quoted lines, call offlineStorage.set('interaction', id, response, result, latency, responseType);: https://github.com/adaptlearning/adapt-contrib-spoor/blob/f07a6032a687b6653c4dbcd903797663fa3ec665/js/adapt-stateful-session.js#L205-L220

Which is calling recordInteraction: https://github.com/adaptlearning/adapt-contrib-spoor/blob/f07a6032a687b6653c4dbcd903797663fa3ec665/js/adapt-offlineStorage-scorm.js#L111-L133

Which is calling isSupported, if (!this.isSupported('cmi.interactions._count')) {: https://github.com/adaptlearning/adapt-contrib-spoor/blob/f07a6032a687b6653c4dbcd903797663fa3ec665/js/scorm/wrapper.js#L366-L392

isSupported calls get: https://github.com/adaptlearning/adapt-contrib-spoor/blob/f07a6032a687b6653c4dbcd903797663fa3ec665/js/scorm/wrapper.js#L475-L490

getValue is called here: https://github.com/adaptlearning/adapt-contrib-spoor/blob/f07a6032a687b6653c4dbcd903797663fa3ec665/libraries/SCORM_API_wrapper.js#L483-L505

Your error is triggering here: https://github.com/adaptlearning/adapt-contrib-spoor/blob/f07a6032a687b6653c4dbcd903797663fa3ec665/js/scorm/wrapper.js#L396-L421

This is your error: https://github.com/adaptlearning/adapt-contrib-spoor/blob/f07a6032a687b6653c4dbcd903797663fa3ec665/js/scorm/error.js#L29

So it is checking, but the check pushes an error to the dialogue.

This is where the dialogue is shown: https://github.com/adaptlearning/adapt-contrib-spoor/blob/f07a6032a687b6653c4dbcd903797663fa3ec665/js/scorm/wrapper.js#L571-L573

It can be controlled with: https://github.com/adaptlearning/adapt-contrib-spoor/blob/f07a6032a687b6653c4dbcd903797663fa3ec665/example.json#L21

However, it does seem a bit odd. isSupported shouldn't really display an error if the value isn't supported.

@danielghost any thoughts?

paulstevendev commented 11 months ago

Surely this code is not checking isSupported('cmi.interactions._count')

https://github.com/adaptlearning/adapt-contrib-spoor/blob/f07a6032a687b6653c4dbcd903797663fa3ec665/js/adapt-stateful-session.js#L213C1-L215C34

const id = this._uniqueInteractionIds ?${this.scorm.getInteractionCount()}-${questionModel.get('_id')} : questionModel.get('_id');

oliverfoster commented 11 months ago

Yup, agreed. Needs an if (!this.scorm.isSupported('cmi.interactions._count')) return; above that line?

paulstevendev commented 11 months ago

That is exactly what I have done and testing as we speak

paulstevendev commented 11 months ago

@oliverfoster happy to create a PR for this.

Not done this before so just checking the procedure

Do I create a local branch issue/294 and then push this up (I don't appear to have permission to do so)

Then create a PR

oliverfoster commented 11 months ago

You can fork the repo, create your issue/294 and then pr from there.

paulstevendev commented 11 months ago

Done

github-actions[bot] commented 11 months ago

:tada: This issue has been resolved in version 5.9.2 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: