Moodle
  1. Moodle
  2. MDL-21332

On entering the 2nd time an AICC HACP course using the "Enter" button w/o checking "Start a new attempt" an error is returned back by the course Java AICC callbacks helper

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.9.7
    • Fix Version/s: 1.9.8
    • Component/s: SCORM
    • Labels:
      None
    • Environment:
      LAMP, PHP 5.2.12
    • Database:
      MySQL
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      26606

      Description

      It seems the Moodle code being not able to find the item to be launched, since everything is working fine when the check box for the new attempt is checked.

        Activity

        Hide
        Matteo Scaramuccia added a comment -

        Fix proposal:

        datamodels/aicclib.php

        @@ -344,6 +344,9 @@
                 $usertracks = array();
                 foreach ($scoes as $sco) {
                     if (!empty($sco->launch)) {
        +                if (!isset($scoid) || $scoid == 0 || $scoid == '' ) {
        +			$scoid = $sco->id;
        +		}
                         if ($usertrack = scorm_get_tracks($sco->id,$user->id,$attempt)) {
                             if ($usertrack->status == '') {
                                 $usertrack->status = 'notattempted';
        
        Show
        Matteo Scaramuccia added a comment - Fix proposal: datamodels/aicclib.php @@ -344,6 +344,9 @@ $usertracks = array(); foreach ($scoes as $sco) { if (!empty($sco->launch)) { + if (!isset($scoid) || $scoid == 0 || $scoid == '' ) { + $scoid = $sco->id; + } if ($usertrack = scorm_get_tracks($sco->id,$user->id,$attempt)) { if ($usertrack->status == '') { $usertrack->status = 'notattempted';
        Hide
        Dan Marsden added a comment -

        FYI - PHP has a nice empty() function that does that line
        this:
        if (!isset($scoid) || $scoid == 0 || $scoid == '' ) {
        becomes this:
        if (empty($scoid)) {

        but I'm not entirely sure on the fix - can you explain a bit more about where $scoid is being called? - what happens when there are more than one sco - does it end up loading on the last sco? - is that correct?

        thanks!

        Show
        Dan Marsden added a comment - FYI - PHP has a nice empty() function that does that line this: if (!isset($scoid) || $scoid == 0 || $scoid == '' ) { becomes this: if (empty($scoid)) { but I'm not entirely sure on the fix - can you explain a bit more about where $scoid is being called? - what happens when there are more than one sco - does it end up loading on the last sco? - is that correct? thanks!
        Hide
        Dan Marsden added a comment -

        ignore that last comment - reading the title of this bug rather than just the content - it makes more sense!

        Show
        Dan Marsden added a comment - ignore that last comment - reading the title of this bug rather than just the content - it makes more sense!
        Hide
        Dan Marsden added a comment -

        fix now in HEAD - would be keen for someone else to review this before it goes into stable - I have very limited AICC objects for testing.

        Show
        Dan Marsden added a comment - fix now in HEAD - would be keen for someone else to review this before it goes into stable - I have very limited AICC objects for testing.
        Hide
        Matteo Scaramuccia added a comment -

        Hi Dan,
        FYI, the proposal avoided the usage of empty() just to underline to the dev team the different values we discovered during our debugging : 0 comes out from the missing (under the conditions in the summary) HTTP GET parameter, scoid from loadSCO.php, retrieved by required_param().

        BTW, using:

        @@ -344,6 +344,9 @@
                 $usertracks = array();
                 foreach ($scoes as $sco) {
                     if (!empty($sco->launch)) {
        +                if (empty($scoid)) {
        +                    $scoid = $sco->id;
        +               }
                         if ($usertrack = scorm_get_tracks($sco->id,$user->id,$attempt)) {
                             if ($usertrack->status == '') {
                                 $usertrack->status = 'notattempted';
        

        on 1.9, the issue is fixed as expected.

        FYI, find below the error returned back on the log w/o the fix:

        [Thu Jan 14 09:29:24 2010] [error] [client 12.34.56.78] PHP Notice:  You have an error in your SQL syntax;
        check the manual that corresponds to your MySQL server version for the right syntax
        to use near 'AND attempt=2 ORDER BY element ASC' at line 1<br /><br />
        
        SELECT * FROM mdl_scorm_scoes_track WHERE userid=334945 AND scoid= AND attempt=2 ORDER BY element ASC
        
        <ul style="text-align: left">
        <li>line 687 of lib/dmllib.php: call to debugging()</li><li>line 609 of lib/dmllib.php: call to get_recordset_sql()</li>
        <li>line 931 of lib/dmllib.php: call to get_recordset_select()</li>
        <li>line 355 of mod/scorm/locallib.php: call to get_records_select()</li>
        <li>line 93 of mod/scorm/player.php: call to scorm_get_tracks()</li>
        </ul>
        
        in /path/to/moodle/lib/weblib.php on line 6962, referer: http://111.222.333.444/moodle/mod/scorm/view.php?id=2243
        

        As you can see a SQL error is arised (AND scoid= AND) so the proposal tries to fix such a big blocking issue, upon which an error, content dependent, will be delivered to the final user.

        Obviously the quality of the fix depends on the level of the comprehension we got in this short time to understand what is the real goal of the discovery of the item to be launched i.e. it could be possible that the proposal doesn't correctly address the original beahviour. This proposal stops the discovery at the first item found to be deliverable.

        Show
        Matteo Scaramuccia added a comment - Hi Dan, FYI, the proposal avoided the usage of empty() just to underline to the dev team the different values we discovered during our debugging : 0 comes out from the missing (under the conditions in the summary) HTTP GET parameter, scoid from loadSCO.php , retrieved by required_param() . BTW, using: @@ -344,6 +344,9 @@ $usertracks = array(); foreach ($scoes as $sco) { if (!empty($sco->launch)) { + if (empty($scoid)) { + $scoid = $sco->id; + } if ($usertrack = scorm_get_tracks($sco->id,$user->id,$attempt)) { if ($usertrack->status == '') { $usertrack->status = 'notattempted'; on 1.9, the issue is fixed as expected. FYI, find below the error returned back on the log w/o the fix: [Thu Jan 14 09:29:24 2010] [error] [client 12.34.56.78] PHP Notice: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'AND attempt=2 ORDER BY element ASC' at line 1<br /><br /> SELECT * FROM mdl_scorm_scoes_track WHERE userid=334945 AND scoid= AND attempt=2 ORDER BY element ASC <ul style= "text-align: left" > <li>line 687 of lib/dmllib.php: call to debugging()</li><li>line 609 of lib/dmllib.php: call to get_recordset_sql()</li> <li>line 931 of lib/dmllib.php: call to get_recordset_select()</li> <li>line 355 of mod/scorm/locallib.php: call to get_records_select()</li> <li>line 93 of mod/scorm/player.php: call to scorm_get_tracks()</li> </ul> in /path/to/moodle/lib/weblib.php on line 6962, referer: http: //111.222.333.444/moodle/mod/scorm/view.php?id=2243 As you can see a SQL error is arised ( AND scoid= AND ) so the proposal tries to fix such a big blocking issue, upon which an error, content dependent, will be delivered to the final user. Obviously the quality of the fix depends on the level of the comprehension we got in this short time to understand what is the real goal of the discovery of the item to be launched i.e. it could be possible that the proposal doesn't correctly address the original beahviour. This proposal stops the discovery at the first item found to be deliverable .
        Hide
        Dan Marsden added a comment -

        fix now in stable as well - thanks for the report and fix!

        Show
        Dan Marsden added a comment - fix now in stable as well - thanks for the report and fix!

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: