Moodle
  1. Moodle
  2. MDL-36853

PHP warnings when returning to an existing attempt

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4.2
    • Component/s: SCORM
    • Labels:
    • Rank:
      46379

      Description

      Replication steps:-

      1. Delpoy attached pack
      2. Attempt the two question as student, donot click "finish".
      3. Click "exist activity" instead
      4. Go back and restart your attempt to see the error.
        Copying my comment from the linked issue:-
      5. I dug a little deeper. The problem happens when you attempt the package, exit the activity after finishing both question but donot click "finish". And than restart the attempt.
      6. There is no error when you finish attempt in the middle of the pack and restart it later.
      7. The error is basically since $result->sco is returned as an array containing a sco object where as the code expects it do be the sco object itself.
      8. Dump of $result without error case:-
        stdClass Object
        (
            [toc] => <div id="scorm_layout">
        <div id="scorm_toc">
        <div id="scorm_tree">
        <ul>
        	<li>
        <a title="a=7&scoid=23&currentorg=ORG-D0F45010CE879ED4C2A2CFD700E3F8F4&attempt=1"><img src="http://ankit.moodle.local/int/master/moodle/theme/image.php/standard/scorm/1353901637/incomplete" alt="Incomplete" title="Incomplete" />&nbsp;Start Quiz&nbsp;</a>	</li>
        </ul>
        </div></div></div>
        <div id="scorm_navpanel"></div>
        
            [toctitle] => Flash Custom (Single SCO)
            [sco] => stdClass Object
                (
                    [id] => 23
                    [scorm] => 7
                    [manifest] => MANIFEST-3A3A393295D01FA6530767F976AF706A
                    [organization] => ORG-D0F45010CE879ED4C2A2CFD700E3F8F4
                    [parent] => ORG-D0F45010CE879ED4C2A2CFD700E3F8F4
                    [identifier] => ITEM-0B4EA308CA525F05ACA9283381611BD9
                    [launch] => test.html
                    [scormtype] => sco
                    [title] => Start Quiz
                    [isvisible] => true
                    [parameters] => 
                    [maxtimeallowed] => 0000:30:00:00
                    [timelimitaction] => continue,no message
                    [masteryscore] => 80
                )
        
            [prerequisites] => 1
            [incomplete] => 1
            [attemptleft] => 1
        )
        
      9. Dump of $result during the error:-
        stdClass Object
        (
            [toc] => <div id="scorm_layout">
        <div id="scorm_toc">
        <div id="scorm_tree">
        <ul>
        	<li>
        <a title="a=6&scoid=21&currentorg=ORG-D0F45010CE879ED4C2A2CFD700E3F8F4&attempt=2"><img src="http://ankit.moodle.local/int/master/moodle/theme/image.php/standard/scorm/1353901637/failed" alt="Failed" title="Failed" />&nbsp;Start Quiz&nbsp;(Score:&nbsp;50)</a>	</li>
        </ul>
        </div></div></div>
        <div id="scorm_navpanel"></div>
        
            [toctitle] => Flash Custom (Single SCO)
            [sco] => Array
                (
                    [0] => stdClass Object
                        (
                            [id] => 21
                            [scorm] => 6
                            [manifest] => MANIFEST-3A3A393295D01FA6530767F976AF706A
                            [organization] => ORG-D0F45010CE879ED4C2A2CFD700E3F8F4
                            [parent] => ORG-D0F45010CE879ED4C2A2CFD700E3F8F4
                            [identifier] => ITEM-0B4EA308CA525F05ACA9283381611BD9
                            [launch] => test.html
                            [scormtype] => sco
                            [title] => Start Quiz
                            [isvisible] => 1
                            [parameters] => 
                            [maxtimeallowed] => 0000:30:00:00
                            [timelimitaction] => continue,no message
                            [masteryscore] => 80
                            [prereq] => 1
                            [statusicon] => <img src="http://ankit.moodle.local/int/master/moodle/theme/image.php/standard/scorm/1353901637/failed" alt="Failed" title="Failed" />
                            [url] => a=6&scoid=21&currentorg=ORG-D0F45010CE879ED4C2A2CFD700E3F8F4&attempt=2
                            [incomplete] => 
                        )
        
                )
        
            [prerequisites] => 1
            [incomplete] => 
            [attemptleft] => 1
        )
        
      10. As pointed out earlier this doesnt seem to have to do anything with the issue in hand, this is a whole different context.

      Thanks

        Issue Links

          Activity

          Hide
          Jamie Smith added a comment - - edited

          It appears that the fix, at least temporarily would be in the locallib.php on line 1832.
          I'm not sure of the programmer's intent. But it appears that when a 'next sco' is not available after the last completed sco the $scoes['scoid'] is returned empty from scorm_get_toc_object leaving the $scoid variable empty as well. When this is the case, it is desirable to simply grab the first sco of the sequence thereby employing '$scoes['scoes'][0]>children[0]'. However, the code used is '$scoes['scoes'][0]>children'. See correction at line 1832 below.

          1832 $result->sco = $scoes['scoes'][0]->children

          SHOULD BE

          1832 $result->sco = $scoes['scoes'][0]->children[0]

          Show
          Jamie Smith added a comment - - edited It appears that the fix, at least temporarily would be in the locallib.php on line 1832. I'm not sure of the programmer's intent. But it appears that when a 'next sco' is not available after the last completed sco the $scoes ['scoid'] is returned empty from scorm_get_toc_object leaving the $scoid variable empty as well. When this is the case, it is desirable to simply grab the first sco of the sequence thereby employing '$scoes ['scoes'] [0] >children [0] '. However, the code used is '$scoes ['scoes'] [0] >children'. See correction at line 1832 below. 1832 $result->sco = $scoes ['scoes'] [0] ->children SHOULD BE 1832 $result->sco = $scoes ['scoes'] [0] ->children [0]
          Hide
          Dan Marsden added a comment -

          thanks Jamie!

          Show
          Dan Marsden added a comment - thanks Jamie!
          Hide
          Dan Marsden added a comment -

          would be good if this could be peer reviewed if possible - I haven't tested it but Jamies suggested patch looks correct to me.

          note for integrators - this is for Moodle 2.4 and master only - it does not affect previous versions.

          Show
          Dan Marsden added a comment - would be good if this could be peer reviewed if possible - I haven't tested it but Jamies suggested patch looks correct to me. note for integrators - this is for Moodle 2.4 and master only - it does not affect previous versions.
          Hide
          Matteo Scaramuccia added a comment -

          Nice job Jamie!

          Show
          Matteo Scaramuccia added a comment - Nice job Jamie!
          Hide
          Ankit Agarwal added a comment -

          Hi Dan & Jamie,
          The patch looks good. I looked at other uses of scorm_get_toc() and I cannot find a place where this might be breaking anything.

          +1 for integration
          Thanks

          Show
          Ankit Agarwal added a comment - Hi Dan & Jamie, The patch looks good. I looked at other uses of scorm_get_toc() and I cannot find a place where this might be breaking anything. +1 for integration Thanks
          Hide
          Dan Marsden added a comment -

          thanks for testing Ankit - bouncing up for integration.

          Show
          Dan Marsden added a comment - thanks for testing Ankit - bouncing up for integration.
          Hide
          Ankit Agarwal added a comment -

          Didn't test, just reviewed

          Show
          Ankit Agarwal added a comment - Didn't test, just reviewed
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          Integrated to 24 and master. Thanks Jamie and Dan

          Show
          Dan Poltawski added a comment - Integrated to 24 and master. Thanks Jamie and Dan
          Hide
          Michael de Raadt added a comment -

          Testing results: Success!

          There were no testing instructions for this issue (rather poor), so I assumed that running the replication steps would be sufficient. I was able to test up to step 4 without seeing any errors.

          Tested in 2.4 and Master.

          Show
          Michael de Raadt added a comment - Testing results: Success! There were no testing instructions for this issue (rather poor), so I assumed that running the replication steps would be sufficient. I was able to test up to step 4 without seeing any errors. Tested in 2.4 and Master.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: