chamilo / chamilo-lms

Chamilo is a learning management system focused on ease of use and accessibility
https://chamilo.org
GNU General Public License v3.0
798 stars 480 forks source link

SCORM 1.2: calling LMSCommit('') clears the stored value for cmi.core.lesson_status #2063

Open moloko opened 7 years ago

moloko commented 7 years ago

I'm one of the lead developers on the Adapt Learning open source e-learning framework - following some queries from a user in our Gitter chat room about SCORM errors occurring with content built using Adapt on the Chamilo LMS, I decided to do a bit of investigation for myself using https://1.10.chamilo.org/

I uploaded some content that was SCORM wrapped but didn't actually track so that I could issue all the necessary commands myself using the developer tools.

What I found was that when you set the lesson_status to 'incomplete' then call LMSCommit to save that change, the next time you ask for the lesson_status within the same session, an empty string is returned. This is not a valid value for lesson_status.

Here's a screenshot of the developer tools showing what I did: chamilo_lesson_status_bug

I also tried again in the next session - as you can see it did successfully record the 'incomplete' status - but, once again, a call to LMSCommit causes it to be 'wiped' from the API: chamilo_issue_session2

moloko commented 7 years ago

I was doing all of this in the 'fullscreen' view mode by the way - using Firefox 54.0.1 (64bit) on Win10

moloko commented 7 years ago

Also just note that although https://1.10.chamilo.org/ isn't the latest version, the person who queried this with us originally says they were using the latest version...

ywarnier commented 7 years ago

Hi @moloko, could you try that on https://11.chamilo.org/ ? (same stuff, just a version 1 year younger and with a few patches to SCORM). Also note (just in case, because it looks like 1.2) that we only support SCORM 1.2.

ywarnier commented 7 years ago

In particular, I believe a few patches I sent 2 days ago might already fix this, and they are available on https://11.chamilo.org/.

Which means "the latest version" the customer uses might not have these patches.

moloko commented 7 years ago

Same result I'm afraid: 2017-07-07 10_22_45-developer tools - https___1 11 chamilo org_courses_aabbb_scorm_adapt_2-1-3_index

kluchrj commented 7 years ago

In main/lp/scorm_api.php line 1410:

/**
 * Reinitializes the SCO's modified variables to an empty list.
 * @return  void
 * @uses The global updatable_vars_list array to register this
 */
function reinit_updatable_vars_list() {
    logit_scorm('Cleaning updatable_vars_list: reinit_updatable_vars_list');
    for (i=0;i < olms.scorm_variables.length;i++) {
        if (olms.updatable_vars_list[olms.scorm_variables[i]]) {
            olms.updatable_vars_list[olms.scorm_variables[i]]=false;
        }
    }
    olms.lesson_status='';
}

Removing the olms.lesson_status=''; seems to resolve this issue for us, but I don't know enough about Chamilo to be certain that this is the correct fix.

jmontoyaa commented 6 years ago

I found the same error with an SCORM here:

https://task.beeznest.com/issues/14609#change-108250

And indeed lesson_status cannot be empty, it should be "not attempted"

According to the pdf:

http://xml.coverpages.org/SCORM-12-RunTimeEnv.pdf

page 3-25

ywarnier commented 6 years ago

I've traced this line back to 9 years ago: https://github.com/chamilo/chamilo-lms/commit/04f68d9c946f65fa26298a9b1e3958d17392191f#diff-5f1de90c462cb937525b9ca0ca5de2c8R738

The only thing I'm worrying about is the different conditions saying `if (olms.lesson_status != '') { ...do something...}. These conditions are not right anymore now and we need to make sure that doesn't break other stuff.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.