Moodle
  1. Moodle
  2. MDL-35868

Error writing to database when previewing quiz with no way to rectify.

    Details

    • Database:
      Any
    • Testing Instructions:
      Hide

      1. Create a simple quiz that allows multiple attempts (e.g. 1 true-false question).
      2. Enrol user u1 in the course as a student.
      3. As user u1, attempt the quiz.
      4. Now change u1's role from Student to Teacher.
      5. As user u1, try to preview the quiz. You should be able to.

      Previously, step 5 caused an error.

      Show
      1. Create a simple quiz that allows multiple attempts (e.g. 1 true-false question). 2. Enrol user u1 in the course as a student. 3. As user u1, attempt the quiz. 4. Now change u1's role from Student to Teacher. 5. As user u1, try to preview the quiz. You should be able to. Previously, step 5 caused an error.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
      mdl35868-moodle24
    • Pull Master Branch:
      mdl35868-master
    • Rank:
      44632

      Description

      Steps to reproduce:

      Switch role to student and attempt a quiz.
      Save all and finish.
      Return to normal role.
      Preview the same quiz.

      You will see the following:
      Debug info: Duplicate entry '1-2-1' for key 'mdl_quizatte_quiuseatt_uix'
      INSERT INTO mdl_quiz_attempts (quiz,userid,preview,layout,attempt,timestart,timefinish,timemodified,state,uniqueid) VALUES(?,?,?,?,?,?,?,?,?,?)
      [array (
      0 => '1',
      1 => '2',
      2 => 1,
      3 => '1,0',
      4 => 1,
      5 => 1349710423,
      6 => 0,
      7 => 1349710423,
      8 => 'inprogress',
      9 => 2,
      )]
      Error code: dmlwriteexception
      Stack trace:
      line 410 of /lib/dml/moodle_database.php: dml_write_exception thrown
      line 1029 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end()
      line 1071 of /lib/dml/mysqli_native_moodle_database.php: call to mysqli_native_moodle_database->insert_record_raw()
      line 251 of /mod/quiz/startattempt.php: call to mysqli_native_moodle_database->insert_record()

      This would be tolerable if you could delete said attempt.
      Even with the latest patch from MDL-35163, you are unable to delete the attempt.

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          You can delete the attempt you made while doing switch role to student.

          In the quiz reports, change it to show 'All users who have attempted the quiz' rather than 'enroled uses ...'.

          And don't attempt the quiz after doing switch role.

          Show
          Tim Hunt added a comment - You can delete the attempt you made while doing switch role to student. In the quiz reports, change it to show 'All users who have attempted the quiz' rather than 'enroled uses ...'. And don't attempt the quiz after doing switch role.
          Hide
          Robert Russo added a comment -

          That simply returns to the enrolled users page and does not delete the attempt.

          Switching back to the all users page shows the attempt again.

          Show
          Robert Russo added a comment - That simply returns to the enrolled users page and does not delete the attempt. Switching back to the all users page shows the attempt again.
          Hide
          Didier Raboud added a comment -

          Hi.

          We've been hit by the same problem and solved it that way:

          Author: Didier Raboud <didier.raboud@liip.ch>
          Date:   Thu Oct 11 15:49:57 2012 +0200
          
              Fix attempts preview tests in case there were already tried.
          
          diff --git a/mod/quiz/startattempt.php b/mod/quiz/startattempt.php
          index 8cead80..edc6f9e 100644
          --- a/mod/quiz/startattempt.php
          +++ b/mod/quiz/startattempt.php
          @@ -105,11 +105,8 @@ if ($lastattempt && ($lastattempt->state == quiz_attempt::IN_PROGRESS ||
           
           } else {
               // Get number for the next or unfinished attempt.
          -    if ($lastattempt && !$lastattempt->preview && !$quizobj->is_preview_user()) {
          -        $attemptnumber = $lastattempt->attempt + 1;
          -    } else {
          +    if (!$lastattempt || $lastattempt->preview || $quizobj->is_preview_user()) {
                   $lastattempt = false;
          -        $attemptnumber = 1;
               }
               $currentattemptid = null;
          

          WDYT?

          Show
          Didier Raboud added a comment - Hi. We've been hit by the same problem and solved it that way: Author: Didier Raboud <didier.raboud@liip.ch> Date: Thu Oct 11 15:49:57 2012 +0200 Fix attempts preview tests in case there were already tried. diff --git a/mod/quiz/startattempt.php b/mod/quiz/startattempt.php index 8cead80..edc6f9e 100644 --- a/mod/quiz/startattempt.php +++ b/mod/quiz/startattempt.php @@ -105,11 +105,8 @@ if ($lastattempt && ($lastattempt->state == quiz_attempt::IN_PROGRESS || } else { // Get number for the next or unfinished attempt. - if ($lastattempt && !$lastattempt->preview && !$quizobj->is_preview_user()) { - $attemptnumber = $lastattempt->attempt + 1; - } else { + if (!$lastattempt || $lastattempt->preview || $quizobj->is_preview_user()) { $lastattempt = false ; - $attemptnumber = 1; } $currentattemptid = null ; WDYT?
          Hide
          Michael Hughes added a comment -

          We're also seeing this on a reasonably regular basis. I'm just wondering if there would be any obvious (and immediate) side effects of deleting the offending row from the databasee(i'm asking to preclude me going off and diving in to the code to try to work it out for myself...And don't shout at me, I'm aware that this would merely be a hacky expedient option and doesn't sort out the underlying causes! )

          Show
          Michael Hughes added a comment - We're also seeing this on a reasonably regular basis. I'm just wondering if there would be any obvious (and immediate) side effects of deleting the offending row from the databasee(i'm asking to preclude me going off and diving in to the code to try to work it out for myself...And don't shout at me, I'm aware that this would merely be a hacky expedient option and doesn't sort out the underlying causes! )
          Hide
          Tim Hunt added a comment -

          If you just delete the quiz_attempts row, you will leave garbage in the database. You really need to delete all the linked data. See the delete_attempt method in mod/quiz/report/attemptsreport.php if you want to do it properly.

          Show
          Tim Hunt added a comment - If you just delete the quiz_attempts row, you will leave garbage in the database. You really need to delete all the linked data. See the delete_attempt method in mod/quiz/report/attemptsreport.php if you want to do it properly.
          Hide
          Salvatore Cordiano added a comment -

          I fix this bug as follow

          // Look for an existing attempt.
          $attempts = quiz_get_user_attempts($quizobj->get_quizid(), $USER->id, 'all', true);
          // comment the following line
          //$lastattempt = end($attempts);
          if(count($attempts)>0)

          { $arrayKeys = array_keys($attempts); $lastattempt = $attempts[$arrayKeys[0]]; }
          Show
          Salvatore Cordiano added a comment - I fix this bug as follow // Look for an existing attempt. $attempts = quiz_get_user_attempts($quizobj->get_quizid(), $USER->id, 'all', true); // comment the following line //$lastattempt = end($attempts); if(count($attempts)>0) { $arrayKeys = array_keys($attempts); $lastattempt = $attempts[$arrayKeys[0]]; }
          Hide
          Adam Olley added a comment - - edited

          We're seeing this more often now as well. It's caused by the introduction of the unique constraint on the userid/quiz/attempt set. Previews are always given attempt=1. Which means if you attempt a quiz, not as a preview. And then go to preview it, you immediately get the error writing to database exception.

          Fix is one of either:
          1. Tell the user they're not allowed to preview the quiz since they've attempted it (I don't think this is truly an option, as teachers would have to fumble about with deleting the attempt they made before they can preview again). If this is the truly desired behaviour, rather than dying it should display a nice message explaining to the user they need to remove their attempt. That or have prevented it in the first place; or

          2. Have the preview created with lastattempt+1, satisfying the constraint.

          Show
          Adam Olley added a comment - - edited We're seeing this more often now as well. It's caused by the introduction of the unique constraint on the userid/quiz/attempt set. Previews are always given attempt=1. Which means if you attempt a quiz, not as a preview. And then go to preview it, you immediately get the error writing to database exception. Fix is one of either: 1. Tell the user they're not allowed to preview the quiz since they've attempted it (I don't think this is truly an option, as teachers would have to fumble about with deleting the attempt they made before they can preview again). If this is the truly desired behaviour, rather than dying it should display a nice message explaining to the user they need to remove their attempt. That or have prevented it in the first place; or 2. Have the preview created with lastattempt+1, satisfying the constraint.
          Hide
          Adam Olley added a comment - - edited

          More repro steps in the accidentally created dupe here: MDL-38778
          Patch also supplied in the duplicate, so up to you Tim Hunt if you wanna deal with it in this item or reopen the dupe and close this one (assuming the approach used in that patch is an adequate solution).

          Show
          Adam Olley added a comment - - edited More repro steps in the accidentally created dupe here: MDL-38778 Patch also supplied in the duplicate, so up to you Tim Hunt if you wanna deal with it in this item or reopen the dupe and close this one (assuming the approach used in that patch is an adequate solution).
          Hide
          Tim Hunt added a comment -

          The code change in MDL-38778 is good, but the commit comment isn't (see http://docs.moodle.org/dev/Commit_cheat_sheet). Therefore the commits need to be amended anyways, so Tim (Lock) please could you switch the issue number to this one, and submit the branches for integration here? Thanks.

          Adam, thanks for spotting the duplicate.

          Show
          Tim Hunt added a comment - The code change in MDL-38778 is good, but the commit comment isn't (see http://docs.moodle.org/dev/Commit_cheat_sheet ). Therefore the commits need to be amended anyways, so Tim (Lock) please could you switch the issue number to this one, and submit the branches for integration here? Thanks. Adam, thanks for spotting the duplicate.
          Hide
          Tim Lock added a comment -

          Links applied.

          Show
          Tim Lock added a comment - Links applied.
          Hide
          Tim Hunt added a comment -

          Great! Thanks Tim, submitting for integration.

          Show
          Tim Hunt added a comment - Great! Thanks Tim, submitting for integration.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi, could you plz add the testing instructions (surely creating them from the "to reproduce" description), so this will be integrated next week? TIA!

          Show
          Eloy Lafuente (stronk7) added a comment - Hi, could you plz add the testing instructions (surely creating them from the "to reproduce" description), so this will be integrated next week? TIA!
          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 -

          Hi Tim (Lock),

          Your master branch is not cleanly based off master - it has other issues in it (and same for the other branches).

          I'm just cherry-picking for now, but it'd be good if this could be avoided for future (its particularly tricky for integrators if there are a number of commits and i'm not sure which is required).

          This is what I got when I tried to pull your changes into master:

          git shortlog ...origin/master
          Dan Poltawski (1):
                Merge branch 'mdl35868-master' of https://github.com/tlock/moodle
          
          Tim Lock (15):
                MDL-34020: Further work on IMS package importing when using Blackboard packages
                MDL-33971: When viewing course log entries for Quiz View Summary a number is displayed not the quiz name
                MDL-27158: Default timezone is incorrect when not set in PHP configuration
                MDL-22669: Moodle bug with course restore section count
                MDL-29077: File resource does not display embedded text file in Internet Explorer
                MDL-34167: Lesson: error submitting no answers for a true false question
                Merge remote-tracking branch 'm2/master'
                MDL-35092: Add proxy support to enrol/paypal IPN
                MDL-36978: Standard Old theme broken if no left blocks configured.
                Merge remote-tracking branch 'm2/master'
                Merge remote-tracking branch 'm2/master'
                MDL-38248: Fix type in Course start date screen
                Revert "MDL-38248: Fix type in Course start date screen"
                Merge remote-tracking branch 'm2/master'
                MDL-35868: quiz: Use correct attemptnumber when a last attempt exists.
          
          Show
          Dan Poltawski added a comment - Hi Tim (Lock), Your master branch is not cleanly based off master - it has other issues in it (and same for the other branches). I'm just cherry-picking for now, but it'd be good if this could be avoided for future (its particularly tricky for integrators if there are a number of commits and i'm not sure which is required). This is what I got when I tried to pull your changes into master: git shortlog ...origin/master Dan Poltawski (1): Merge branch 'mdl35868-master' of https: //github.com/tlock/moodle Tim Lock (15): MDL-34020: Further work on IMS package importing when using Blackboard packages MDL-33971: When viewing course log entries for Quiz View Summary a number is displayed not the quiz name MDL-27158: Default timezone is incorrect when not set in PHP configuration MDL-22669: Moodle bug with course restore section count MDL-29077: File resource does not display embedded text file in Internet Explorer MDL-34167: Lesson: error submitting no answers for a true false question Merge remote-tracking branch 'm2/master' MDL-35092: Add proxy support to enrol/paypal IPN MDL-36978: Standard Old theme broken if no left blocks configured. Merge remote-tracking branch 'm2/master' Merge remote-tracking branch 'm2/master' MDL-38248: Fix type in Course start date screen Revert "MDL-38248: Fix type in Course start date screen" Merge remote-tracking branch 'm2/master' MDL-35868: quiz: Use correct attemptnumber when a last attempt exists.
          Hide
          Dan Poltawski added a comment -

          Cherry-picked to master, 24 and 23 - thanks!

          Show
          Dan Poltawski added a comment - Cherry-picked to master, 24 and 23 - thanks!
          Hide
          Ankit Agarwal added a comment -

          Works as described, was able to preview.
          Thanks

          Show
          Ankit Agarwal added a comment - Works as described, was able to preview. Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Your awesome contributions are now part of Moodle, your fav LMS out there.

          Closing this as fixed.

          Many thanks for all the hard work, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Your awesome contributions are now part of Moodle, your fav LMS out there. Closing this as fixed. Many thanks for all the hard work, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: