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:

      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

        Gliffy Diagrams

          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: