Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-40095

SCORM update + completion options locked = error/moodle/dmlwriteexception

    Details

    • Testing Instructions:
      Hide
      1. Enable Site administration ► Advanced features ► Completion tracking
      2. Enable completion tracking on one of your courses or create a new one from scratch
      3. Add a SCORM package into that course, using DnD. In case you're missing a working SCORM package, you can use the Prodding SCO and paste the script below in the Scripted test tab, selecting 200 for Interval between calls:

        LMSInitialize("")
        SetValue("cmi.core.session_time","00:04:59")
        SetValue("cmi.core.lesson_status","completed")
        LMSFinish("")
        

        More detail about this testing&debugging tool here.

      4. Edit the SCORM activity adding the required description (missing here due to DnD), changing Completion tracking into Show activity as complete when conditions are met and checking Passed in Require status. Click Save and return to course
      5. Attend to it and pass it. Completing it will be OK only if you have selected Completed in the step above
      6. Edit the activity settings again, check that Completion options locked will appear, add some text to the description and click save without any further change: no error should appear
      Show
      Enable Site administration ► Advanced features ► Completion tracking Enable completion tracking on one of your courses or create a new one from scratch Add a SCORM package into that course, using DnD. In case you're missing a working SCORM package, you can use the Prodding SCO and paste the script below in the Scripted test tab, selecting 200 for Interval between calls : LMSInitialize("") SetValue("cmi.core.session_time","00:04:59") SetValue("cmi.core.lesson_status","completed") LMSFinish("") More detail about this testing&debugging tool here . Edit the SCORM activity adding the required description (missing here due to DnD), changing Completion tracking into Show activity as complete when conditions are met and checking Passed in Require status . Click Save and return to course Attend to it and pass it. Completing it will be OK only if you have selected Completed in the step above Edit the activity settings again, check that Completion options locked will appear, add some text to the description and click save without any further change: no error should appear
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      m26_MDL-40095_Completion_locked_Error_on_save

      Description

      When a scorm package is set with activity completion based on "Require status" condition and this activity completion is locked (a student marked this activity as completed) it is impossible to update the activity.
      Indeed, in this conditions any parameter modification (even minor as the description) generate an error (error/moodle/dmlwriteexception) when you save it.
      When debugging is activated, it seems that is the DB update request which is malformed.
      The "completionstatusrequired" field is set with an array instead an integer.

      See the screen shot in attachment for more details.

      Note that if you lock the activity completion settings with a condition on a note for example there is no problem.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              toinoo antoine faget added a comment -

              Error trace screenshot: problem is on the completionstatusrequired set with an array
              23 => array (4 => 1,) instead an integer

              Show
              toinoo antoine faget added a comment - Error trace screenshot: problem is on the completionstatusrequired set with an array 23 => array (4 => 1,) instead an integer
              Hide
              matteo Matteo Scaramuccia added a comment -

              For the record: something similar has been already posted in the SCORM Community, https://moodle.org/mod/forum/discuss.php?d=229514.

              Show
              matteo Matteo Scaramuccia added a comment - For the record: something similar has been already posted in the SCORM Community, https://moodle.org/mod/forum/discuss.php?d=229514 .
              Hide
              quen Sam Marshall added a comment - - edited

              Whoever looks after SCORM, please investigate this - I don't think this is a generic problem with locking completion settings; the 'completionstatusrequired' field appears to be part of SCORM and not part of the completion system.

              That said it might be related to a fix to completion locking in general (for all activities) which went live recently, MDL-38315.

              Show
              quen Sam Marshall added a comment - - edited Whoever looks after SCORM, please investigate this - I don't think this is a generic problem with locking completion settings; the 'completionstatusrequired' field appears to be part of SCORM and not part of the completion system. That said it might be related to a fix to completion locking in general (for all activities) which went live recently, MDL-38315 .
              Hide
              matteo Matteo Scaramuccia added a comment -

              Spare time permitted, I'll look at the issue proposing a patch closed to the SCORM activity, where in some circumstances $data->completionstatusrequired keeps on being an array e.g. array ( 4 => 1 ) (indicating completed) as per the example attached, breaking the Save functionality.
              @Sam Marshall: in case of regression, I'll add the label.

              Show
              matteo Matteo Scaramuccia added a comment - Spare time permitted, I'll look at the issue proposing a patch closed to the SCORM activity, where in some circumstances $data->completionstatusrequired keeps on being an array e.g. array ( 4 => 1 ) (indicating completed ) as per the example attached, breaking the Save functionality. @ Sam Marshall : in case of regression, I'll add the label.
              Hide
              matteo Matteo Scaramuccia added a comment -

              Hi Dan,
              please review my proposal: testing instructions will follow.

              Show
              matteo Matteo Scaramuccia added a comment - Hi Dan, please review my proposal: testing instructions will follow.
              Hide
              danmarsden Dan Marsden added a comment -

              looks ok to me - just some extra spaces in the if:
              if ( isset($da....

              thanks,

              Show
              danmarsden Dan Marsden added a comment - looks ok to me - just some extra spaces in the if: if ( isset($da.... thanks,
              Hide
              matteo Matteo Scaramuccia added a comment -

              Hi Dan,
              they were there to kept aligned the conditions: I'll remove them my evening.

              Show
              matteo Matteo Scaramuccia added a comment - Hi Dan, they were there to kept aligned the conditions: I'll remove them my evening.
              Hide
              matteo Matteo Scaramuccia added a comment -

              Hi Dan,
              back again: extra spaces removed and put back into a single line, as already coded before MDL-38315 commit.

              Show
              matteo Matteo Scaramuccia added a comment - Hi Dan, back again: extra spaces removed and put back into a single line, as already coded before MDL-38315 commit.
              Hide
              danmarsden Dan Marsden added a comment -

              looks fine to me - submitting for integration. - Thanks Matteo!

              Show
              danmarsden Dan Marsden added a comment - looks fine to me - submitting for integration. - Thanks Matteo!
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks guys, this has been integrated now

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks guys, this has been integrated now
              Hide
              skodak Petr Skoda added a comment -

              I am sorry, I am unable to test this because I did not find any scorm package where I could "Attend to it and pass it.", sorry.

              Show
              skodak Petr Skoda added a comment - I am sorry, I am unable to test this because I did not find any scorm package where I could "Attend to it and pass it.", sorry.
              Hide
              matteo Matteo Scaramuccia added a comment -

              Hi Petr,
              just updated the testing instructions to provide a sample for start with testing: I was wrongly guessing that a tester assigned to an issue related to the SCORM component already have AICC&SCORM packages as part of the fixtures.

              Show
              matteo Matteo Scaramuccia added a comment - Hi Petr, just updated the testing instructions to provide a sample for start with testing: I was wrongly guessing that a tester assigned to an issue related to the SCORM component already have AICC&SCORM packages as part of the fixtures.
              Hide
              skodak Petr Skoda added a comment -

              Thanks, works fine for me!

              Show
              skodak Petr Skoda added a comment - Thanks, works fine for me!
              Hide
              matteo Matteo Scaramuccia added a comment -

              Hi Petr,
              cool !
              Did you miss to change the status? It still say: Problem during testing.

              TIA,
              Matteo

              Show
              matteo Matteo Scaramuccia added a comment - Hi Petr, cool ! Did you miss to change the status? It still say: Problem during testing . TIA, Matteo
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Marking as passed.

              Show
              samhemelryk Sam Hemelryk added a comment - Marking as passed.
              Hide
              matteo Matteo Scaramuccia added a comment -

              TNX Sam!

              Show
              matteo Matteo Scaramuccia added a comment - TNX Sam!
              Hide
              gfox Graham Fox added a comment -

              This is a pretty major minor bug. Can be a big problem if say 500 people have gone through a SCORM file and a small change needs to be made like updating the description or the file display size.

              Show
              gfox Graham Fox added a comment - This is a pretty major minor bug. Can be a big problem if say 500 people have gone through a SCORM file and a small change needs to be made like updating the description or the file display size.
              Hide
              poltawski Dan Poltawski added a comment -

              Thanks for your contributions!

              _main:
              @ BB#0:
                      push    {r7, lr}
                      mov     r7, sp
                      sub     sp, #4
                      movw    r0, :lower16:(L_.str-(LPC0_0+4))
                      movt    r0, :upper16:(L_.str-(LPC0_0+4))
              LPC0_0:
                      add     r0, pc
                      bl      _printf
                      movs    r1, #0
                      movt    r1, #0
                      str     r0, [sp]                @ 4-byte Spill
                      mov     r0, r1
                      add     sp, #4
                      pop     {r7, pc}
               
                      .section        __TEXT,__cstring,cstring_literals
              L_.str:                                 @ @.str
                      .asciz   "This code is now upstream!"
              

              Show
              poltawski Dan Poltawski added a comment - Thanks for your contributions! _main: @ BB#0: push {r7, lr} mov r7, sp sub sp, #4 movw r0, :lower16:(L_.str-(LPC0_0+4)) movt r0, :upper16:(L_.str-(LPC0_0+4)) LPC0_0: add r0, pc bl _printf movs r1, #0 movt r1, #0 str r0, [sp] @ 4-byte Spill mov r0, r1 add sp, #4 pop {r7, pc}   .section __TEXT,__cstring,cstring_literals L_.str: @ @.str .asciz "This code is now upstream!"

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    8/Jul/13