Moodle
  1. Moodle
  2. MDL-26672

A value equal to 0 is meaningful within the context of the AICC HACP Mastery Score business logic

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.11, 2.0.2
    • Fix Version/s: 2.0.3
    • Component/s: SCORM
    • Labels:
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      16506

      Description

      On making a deep analysis on a significant (for us ) number of records about an unexpected SCO status found in the mdl_scorm_scoes_track of a medium-sized Moodle installation with plenty of AICC HACP based packages, we found a flaw in the business logic related with the evaluation of the status by the Mastery Score mechanism.

      Shortly, the failure is due to a bad, in this case, usage of PHP empty() which excludes 0 from being a valid value.

        Activity

        Hide
        Matteo Scaramuccia added a comment -

        Find below the patch we're thinking to test. Basically:

        • removed the emtpy() calls using a different sanity check strategy trying the usage of is_numeric(), already used in checking a score;
        • improved the Mastery_Score initilization in replying to a GetParam command.
        Index: aicc.php
        ===================================================================
        RCS file: /cvsroot/moodle/moodle/mod/scorm/aicc.php,v
        retrieving revision 1.21.2.6
        diff -u -r1.21.2.6 aicc.php
        --- aicc.php	6 May 2010 00:13:54 -0000	1.21.2.6
        +++ aicc.php	3 Mar 2011 14:14:42 -0000
        @@ -77,7 +77,7 @@
                                 if ($sco = scorm_get_sco($scoid)) {
                                     $userdata->course_id = $sco->identifier;
                                     $userdata->datafromlms = isset($sco->datafromlms)?$sco->datafromlms:'';
        -							$userdata->mastery_score = isset($sco->mastery_score)?$sco->mastery_score:'';
        +                            $userdata->mastery_score = isset($sco->mastery_score) && is_numeric($sco->mastery_score)?trim($sco->mastery_score):'';
                                     $userdata->max_time_allowed = isset($sco->max_time_allowed)?$sco->max_time_allowed:'';
                                     $userdata->time_limit_action = isset($sco->time_limit_action)?$sco->time_limit_action:'';
                                     echo "error=0\r\nerror_text=Successful\r\naicc_data=";
        @@ -260,11 +260,10 @@
                                         $id = scorm_insert_track($USER->id, $scorm->id, $sco->id, $attempt, 'cmi.core.lesson_status', 'browsed');
                                     }
                                     if ($mode == 'normal') {
        -
                                         if ($sco = scorm_get_sco($scoid)) {
        -                                    if (!empty($sco->mastery_score)) {
        -                                        if (!empty($score)) {
        -                                            if ($score >= $sco->mastery_score) {
        +                                    if (isset($sco->mastery_score) && is_numeric($sco->mastery_score)) {
        +                                        if ($score != '') { // $score is correctly initialized w/ an empty string, see above
        +                                            if ($score >= trim($sco->mastery_score)) {
                                                         $lessonstatus = 'passed';
                                                     } else {
                                                         $lessonstatus = 'failed';
        

        Note: is_numeric() is effective only in conjunction with trim(), see below.

        <?php
        header('Content-Type: text/plain');
        
        $values = array(
            ' 75',
            '75 ',
            '75',
            'aaa',
            'Llll',
            '111',
            '111L',
            ''
        );
        
        echo 'PHP v'.phpversion()."\n";
        $i = 0;
        foreach ($values as $value) {
            $i++;
            echo "#$i,\t'$value'\t=> ".(is_numeric($value)?'is_num':'not_is_num')."\n";
        }
        

        which outputs:

        PHP v5.2.14
        #1,	' 75'	=> is_num	<===== Here is where trim() is required
        #2,	'75 '	=> not_is_num
        #3,	'75'	=> is_num
        #4,	'aaa'	=> not_is_num
        #5,	'Llll'	=> not_is_num
        #6,	'111'	=> is_num
        #7,	'111L'	=> not_is_num
        #8,	''	=> not_is_num
        
        Show
        Matteo Scaramuccia added a comment - Find below the patch we're thinking to test. Basically: removed the emtpy() calls using a different sanity check strategy trying the usage of is_numeric() , already used in checking a score; improved the Mastery_Score initilization in replying to a GetParam command. Index: aicc.php =================================================================== RCS file: /cvsroot/moodle/moodle/mod/scorm/aicc.php,v retrieving revision 1.21.2.6 diff -u -r1.21.2.6 aicc.php --- aicc.php 6 May 2010 00:13:54 -0000 1.21.2.6 +++ aicc.php 3 Mar 2011 14:14:42 -0000 @@ -77,7 +77,7 @@ if ($sco = scorm_get_sco($scoid)) { $userdata->course_id = $sco->identifier; $userdata->datafromlms = isset($sco->datafromlms)?$sco->datafromlms:''; - $userdata->mastery_score = isset($sco->mastery_score)?$sco->mastery_score:''; + $userdata->mastery_score = isset($sco->mastery_score) && is_numeric($sco->mastery_score)?trim($sco->mastery_score):''; $userdata->max_time_allowed = isset($sco->max_time_allowed)?$sco->max_time_allowed:''; $userdata->time_limit_action = isset($sco->time_limit_action)?$sco->time_limit_action:''; echo "error=0\r\nerror_text=Successful\r\naicc_data=" ; @@ -260,11 +260,10 @@ $id = scorm_insert_track($USER->id, $scorm->id, $sco->id, $attempt, 'cmi.core.lesson_status', 'browsed'); } if ($mode == 'normal') { - if ($sco = scorm_get_sco($scoid)) { - if (!empty($sco->mastery_score)) { - if (!empty($score)) { - if ($score >= $sco->mastery_score) { + if (isset($sco->mastery_score) && is_numeric($sco->mastery_score)) { + if ($score != '') { // $score is correctly initialized w/ an empty string, see above + if ($score >= trim($sco->mastery_score)) { $lessonstatus = 'passed'; } else { $lessonstatus = 'failed'; Note: is_numeric() is effective only in conjunction with trim() , see below. <?php header('Content-Type: text/plain'); $values = array( ' 75', '75 ', '75', 'aaa', 'Llll', '111', '111L', '' ); echo 'PHP v'.phpversion(). "\n" ; $i = 0; foreach ($values as $value) { $i++; echo "#$i,\t'$value'\t=> " .(is_numeric($value)?'is_num':'not_is_num'). "\n" ; } which outputs: PHP v5.2.14 #1, ' 75' => is_num <===== Here is where trim() is required #2, '75 ' => not_is_num #3, '75' => is_num #4, 'aaa' => not_is_num #5, 'Llll' => not_is_num #6, '111' => is_num #7, '111L' => not_is_num #8, '' => not_is_num
        Hide
        Dan Marsden added a comment -

        thanks Matteo - all makes sense to me. Will try to create a pull request early next week for this.

        Show
        Dan Marsden added a comment - thanks Matteo - all makes sense to me. Will try to create a pull request early next week for this.
        Hide
        Helen Foster added a comment -

        Thanks Matteo and Dan.

        Adding a fix version of stable backlog as part of our triaging process.

        Show
        Helen Foster added a comment - Thanks Matteo and Dan. Adding a fix version of stable backlog as part of our triaging process.
        Hide
        Dan Marsden added a comment -

        pull request has been submitted - thanks Matteo!

        Show
        Dan Marsden added a comment - pull request has been submitted - thanks Matteo!
        Hide
        Matteo Scaramuccia added a comment -

        I'm glad I could help

        Show
        Matteo Scaramuccia added a comment - I'm glad I could help
        Hide
        Helen Foster added a comment -

        This issue is fixed in the latest 2.0.2+. Thanks everyone.

        Show
        Helen Foster added a comment - This issue is fixed in the latest 2.0.2+. Thanks everyone.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: