jcputney / scorm-again

A modern SCORM JavaScript runtime library.
https://jcputney.github.io/scorm-again/
MIT License
221 stars 54 forks source link

Issue: loadFromFlattenedJSON creating invalid cmi key #587

Closed techyian closed 1 month ago

techyian commented 1 year ago

Hi, first I'd like to say well done for this library, I'm currently trying to migrate our current SCORM 1.2 runtime over to your library and I've come across what I think is an issue, but it may be my understanding which is incorrect.

Our e-learning is created with Articulate Storyline generally, and I'm trying to load in the cmi.suspend_data value to the API, which from your docs state it's possible via the loadFromFlattenedJSON or loadFromJSON functions for which I've used the former. I'm using the following to load in the initial location value:

window.API.loadFromFlattenedJSON({
        'cmi.suspend_data': 'the location string'
});

Following the code in DevTools shows that when it eventually enters loadFromJSON, the object has taken the form cmi: {suspend_data: 'the location string'} which I think is correct at this stage. Where things get a bit wonky is where you're setting the default value for the CMIElement variable as cmi (line 1025 in BaseAPI.js) because when the JSON object is processed, you're then doing const currentCMIElement = (CMIElement ? CMIElement + '.' : '') + key; on line 1032, but CMIElement will never be undefined at this stage and this then results in currentCMIElement taking the form cmi.cmi.

I can get around this by doing the following but ideally there needs to be a fix put in place for this, unless of course I've messed up somewhere:

window.API.loadFromFlattenedJSON({
        'suspend_data': 'the location string'
}, 'cmi');

If it is indeed a bug, I'm happy to send a PR over for a fix but would prefer confirmation before doing so.

Thanks again for your great efforts,

Ian.

jcputney commented 1 year ago

@techyian Hmmm, yes, that does appear to be a bug. I would be happy to take a look at a PR, and if you wouldn't mind a test case for this as well. Thanks!

ngocducdim commented 1 year ago

Is there any fix yet? this is a big issue when we can't load the recorded data

ngocducdim commented 1 year ago

Temporary fix for this issue(typescript)

function fixSavedData(savedData: any) {
  const fixed: any = {};
  for (const key in savedData) {
    const formatedKey = key as string;
    if (formatedKey.startsWith('cmi.')) {
      const fixedKey = formatedKey.replace('cmi.', '');
      fixed[fixedKey] = savedData[formatedKey];
    } else {
      fixed[formatedKey] = savedData;
    }
  }
  return fixed;
}
ngocducdim commented 1 year ago

Also there is a bug when loading cmi.objectives, using this data:

{"cmi.suspend_data":"","cmi.launch_data":"","cmi.comments":"","cmi.comments_from_lms":"","cmi.core.student_id":"student1","cmi.core.student_name":"Student 1","cmi.core.lesson_location":"topic-nOmtUy","cmi.core.credit":"","cmi.core.lesson_status":"completed","cmi.core.entry":"","cmi.core.lesson_mode":"normal","cmi.core.exit":"","cmi.core.session_time":"00:03:50","cmi.core.score.raw":"","cmi.core.score.min":"","cmi.core.score.max":"100","cmi.core.total_time":"00:03:50","cmi.objectives.0.id":"topic-aMMlFF","cmi.objectives.0.status":"completed","cmi.objectives.0.score.raw":"","cmi.objectives.0.score.min":"","cmi.objectives.0.score.max":"100","cmi.objectives.1.id":"topic-6ReD72","cmi.objectives.1.status":"completed","cmi.objectives.1.score.raw":"","cmi.objectives.1.score.min":"","cmi.objectives.1.score.max":"100","cmi.objectives.2.id":"topic-sfvmuO","cmi.objectives.2.status":"completed","cmi.objectives.2.score.raw":"","cmi.objectives.2.score.min":"","cmi.objectives.2.score.max":"100","cmi.objectives.3.id":"topic-IObzTr","cmi.objectives.3.status":"completed","cmi.objectives.3.score.raw":"","cmi.objectives.3.score.min":"","cmi.objectives.3.score.max":"100","cmi.objectives.4.id":"topic-Bq3O5v","cmi.objectives.4.status":"completed","cmi.objectives.4.score.raw":"","cmi.objectives.4.score.min":"","cmi.objectives.4.score.max":"100","cmi.objectives.5.id":"topic-HmnDxg","cmi.objectives.5.status":"completed","cmi.objectives.5.score.raw":"","cmi.objectives.5.score.min":"","cmi.objectives.5.score.max":"100","cmi.objectives.6.id":"topic-4YswmY","cmi.objectives.6.status":"completed","cmi.objectives.6.score.raw":"","cmi.objectives.6.score.min":"","cmi.objectives.6.score.max":"100","cmi.objectives.7.id":"topic-LuAS5e","cmi.objectives.7.status":"completed","cmi.objectives.7.score.raw":"","cmi.objectives.7.score.min":"","cmi.objectives.7.score.max":"100","cmi.objectives.8.id":"topic-K6ECXa","cmi.objectives.8.status":"completed","cmi.objectives.8.score.raw":"","cmi.objectives.8.score.min":"","cmi.objectives.8.score.max":"100","cmi.objectives.9.id":"topic-2lpxvN","cmi.objectives.9.status":"completed","cmi.objectives.9.score.raw":"","cmi.objectives.9.score.min":"","cmi.objectives.9.score.max":"100","cmi.objectives.10.id":"topic-MllWvr","cmi.objectives.10.status":"completed","cmi.objectives.10.score.raw":"","cmi.objectives.10.score.min":"","cmi.objectives.10.score.max":"100","cmi.objectives.11.id":"topic-m0dnP3","cmi.objectives.11.status":"completed","cmi.objectives.11.score.raw":"","cmi.objectives.11.score.min":"","cmi.objectives.11.score.max":"100","cmi.objectives.12.id":"topic-so4hKC","cmi.objectives.12.status":"completed","cmi.objectives.12.score.raw":"","cmi.objectives.12.score.min":"","cmi.objectives.12.score.max":"100","cmi.objectives.13.id":"topic-S9p8FE","cmi.objectives.13.status":"completed","cmi.objectives.13.score.raw":"","cmi.objectives.13.score.min":"","cmi.objectives.13.score.max":"100","cmi.objectives.14.id":"topic-E97B5s","cmi.objectives.14.status":"completed","cmi.objectives.14.score.raw":"","cmi.objectives.14.score.min":"","cmi.objectives.14.score.max":"100","cmi.objectives.15.id":"topic-TTyEf0","cmi.objectives.15.status":"completed","cmi.objectives.15.score.raw":"","cmi.objectives.15.score.min":"","cmi.objectives.15.score.max":"100","cmi.objectives.16.id":"topic-Bk8ZLK","cmi.objectives.16.status":"completed","cmi.objectives.16.score.raw":"","cmi.objectives.16.score.min":"","cmi.objectives.16.score.max":"100","cmi.objectives.17.id":"topic-5rxzyV","cmi.objectives.17.status":"completed","cmi.objectives.17.score.raw":"","cmi.objectives.17.score.min":"","cmi.objectives.17.score.max":"100","cmi.objectives.18.id":"topic-Q3rjln","cmi.objectives.18.status":"completed","cmi.objectives.18.score.raw":"","cmi.objectives.18.score.min":"","cmi.objectives.18.score.max":"100","cmi.objectives.19.id":"topic-eIbwi4","cmi.objectives.19.status":"completed","cmi.objectives.19.score.raw":"","cmi.objectives.19.score.min":"","cmi.objectives.19.score.max":"100","cmi.objectives.20.id":"topic-AzAspy","cmi.objectives.20.status":"completed","cmi.objectives.20.score.raw":"","cmi.objectives.20.score.min":"","cmi.objectives.20.score.max":"100","cmi.objectives.21.id":"topic-ehMV4A","cmi.objectives.21.status":"completed","cmi.objectives.21.score.raw":"","cmi.objectives.21.score.min":"","cmi.objectives.21.score.max":"100","cmi.objectives.22.id":"topic-U52hDp","cmi.objectives.22.status":"completed","cmi.objectives.22.score.raw":"","cmi.objectives.22.score.min":"","cmi.objectives.22.score.max":"100","cmi.objectives.23.id":"topic-mmAPuC","cmi.objectives.23.status":"completed","cmi.objectives.23.score.raw":"","cmi.objectives.23.score.min":"","cmi.objectives.23.score.max":"100","cmi.objectives.24.id":"topic-UyqqeN","cmi.objectives.24.status":"completed","cmi.objectives.24.score.raw":"","cmi.objectives.24.score.min":"","cmi.objectives.24.score.max":"100","cmi.objectives.25.id":"topic-UIALBn","cmi.objectives.25.status":"completed","cmi.objectives.25.score.raw":"","cmi.objectives.25.score.min":"","cmi.objectives.25.score.max":"100","cmi.objectives.26.id":"topic-oLGCQE","cmi.objectives.26.status":"completed","cmi.objectives.26.score.raw":"","cmi.objectives.26.score.min":"","cmi.objectives.26.score.max":"100","cmi.objectives.27.id":"topic-JhRmY0","cmi.objectives.27.status":"completed","cmi.objectives.27.score.raw":"","cmi.objectives.27.score.min":"","cmi.objectives.27.score.max":"100","cmi.objectives.28.id":"topic-677fLH","cmi.objectives.28.status":"completed","cmi.objectives.28.score.raw":"","cmi.objectives.28.score.min":"","cmi.objectives.28.score.max":"100","cmi.objectives.29.id":"topic-rKvnIL","cmi.objectives.29.status":"completed","cmi.objectives.29.score.raw":"","cmi.objectives.29.score.min":"","cmi.objectives.29.score.max":"100","cmi.objectives.30.id":"topic-Vjd6mO","cmi.objectives.30.status":"completed","cmi.objectives.30.score.raw":"","cmi.objectives.30.score.min":"","cmi.objectives.30.score.max":"100","cmi.objectives.31.id":"topic-jUsEtX","cmi.objectives.31.status":"completed","cmi.objectives.31.score.raw":"","cmi.objectives.31.score.min":"","cmi.objectives.31.score.max":"100","cmi.objectives.32.id":"topic-SvCWWr","cmi.objectives.32.status":"completed","cmi.objectives.32.score.raw":"","cmi.objectives.32.score.min":"","cmi.objectives.32.score.max":"100","cmi.objectives.33.id":"topic-knRFfG","cmi.objectives.33.status":"completed","cmi.objectives.33.score.raw":"","cmi.objectives.33.score.min":"","cmi.objectives.33.score.max":"100","cmi.objectives.34.id":"topic-wlFwhf","cmi.objectives.34.status":"completed","cmi.objectives.34.score.raw":"","cmi.objectives.34.score.min":"","cmi.objectives.34.score.max":"100","cmi.objectives.35.id":"topic-3b86fq","cmi.objectives.35.status":"completed","cmi.objectives.35.score.raw":"","cmi.objectives.35.score.min":"","cmi.objectives.35.score.max":"100","cmi.objectives.36.id":"topic-ZjIFKf","cmi.objectives.36.status":"completed","cmi.objectives.36.score.raw":"","cmi.objectives.36.score.min":"","cmi.objectives.36.score.max":"100","cmi.objectives.37.id":"topic-A9spZz","cmi.objectives.37.status":"completed","cmi.objectives.37.score.raw":"","cmi.objectives.37.score.min":"","cmi.objectives.37.score.max":"100","cmi.objectives.38.id":"topic-nOmtUy","cmi.objectives.38.status":"completed","cmi.objectives.38.score.raw":"","cmi.objectives.38.score.min":"","cmi.objectives.38.score.max":"100","cmi.objectives.39.id":"topic-jxtabl","cmi.objectives.39.status":"completed","cmi.objectives.39.score.raw":"","cmi.objectives.39.score.min":"","cmi.objectives.39.score.max":"100","cmi.student_data.mastery_score":"","cmi.student_data.max_time_allowed":"","cmi.student_data.time_limit_action":"","cmi.student_preference.audio":"","cmi.student_preference.language":"","cmi.student_preference.speed":"","cmi.student_preference.text":"","cmi.interactions":{}}

after load the saved data using loadFromFlattenedJSON then get the value cmi.objectives.10.id it will return empty string, but the saved data have, maybe occur when lib parse the saved data

github-actions[bot] commented 1 month ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

jcputney commented 1 month ago

@ngocducdim I am no longer able to reproduce the issue after the 2.0.0 release, and I have added a test case for the specific data that you sent. Let me know if you're still seeing the issue. If so, please create a separate ticket.

@techyian I took another look, and the reason the code seems a little off is because the line

const currentCMIElement = (CMIElement ? CMIElement + '.' : '') + key;

is still valid when CMIElement === '', which is what I'm passing to loadFromFlattenedJSON as the root element.

I think that explains your issue with the code, and I think any other issues listed above should be fixed. If not, feel free to open another ticket.