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

A database transaction in quiz editing does not enclose all the relevant DB queries

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      The but only manifest with DB read-write splitting (as in MDL-19711). This test just ensures that the change does not introduce regressions. The automated tests provider further confidence about that.

      1. Login as an admin.
      2. Create a course.
      3. Create a quiz.
      4. Add 2 questions to question bank (`question 1` and `question 2`).
      5. Edit the quiz.
      6. Click `Add` -> `From question bank` and select questions create in 4.
      7. Click `Add selected questions to the quiz`.
      8. Confirm, that both questions have been added to the quiz and no error is displayed.

       

      Show
      The but only manifest with DB read-write splitting (as in MDL-19711 ). This test just ensures that the change does not introduce regressions. The automated tests provider further confidence about that. Login as an admin. Create a course. Create a quiz. Add 2 questions to question bank (`question 1` and `question 2`). Edit the quiz. Click `Add` -> `From question bank` and select questions create in 4. Click `Add selected questions to the quiz`. Confirm , that both questions have been added to the quiz and no error is displayed.  
    • Affected Branches:
      MOODLE_37_STABLE, MOODLE_38_STABLE, MOODLE_39_STABLE
    • Fixed Branches:
      MOODLE_37_STABLE, MOODLE_38_STABLE
    • Pull from Repository:
    • Pull 3.7 Branch:
      MOODLE_37_MDL-67540
    • Pull 3.8 Branch:
      MOODLE_38_MDL-67540
    • Pull Master Branch:
      master_MDL-67540

      Description

      We noticed this bug as our client is setup with some database read-only slaves, but I believe the same issue could occur if two users happened to be trying to do this at the same time. 

      The users were attempting to add multiple quiz questions in one go (through the web-interface and through an external 'Respondus' tool). The first question was always added, but then they were met with a database error, saying the was already a unique key for that quiz id and slot id.

      The issue boils down to the fact that in the database query to check the existing slots on the quiz, happens before the database transaction begins. 

      $slots = $DB->get_records('quiz_slots', array('quizid' => $quiz->id), 'slot', 'questionid, slot, page, id');
       
      if (array_key_exists($questionid, $slots)) {
      return false
      } 
       
      $trans = $DB->start_delegated_transaction(); 

        

      To fix this, the transaction should simply be moved above the get_records call.

       
      I'm attaching a .diff file. Let me know if you want me to actually create a github branch for it.

        Attachments

        1. client-WR325044.diff
          0.7 kB
          C Warwicker
        2. MDL-67540_Race_1.jpg
          137 kB
          Mikhail Golenkov
        3. MDL-67540_Race_2.jpg
          150 kB
          Mikhail Golenkov

          Issue Links

            Activity

              People

              Assignee:
              mikhailgolenkov Mikhail Golenkov
              Reporter:
              cwarwicker C Warwicker
              Peer reviewer:
              Tim Hunt
              Integrator:
              Andrew Nicols
              Tester:
              CiBoT
              Participants:
              Component watchers:
              Tim Hunt, Andrew Nicols, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze, Tim Hunt, Andrew Nicols, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
              Votes:
              1 Vote for this issue
              Watchers:
              9 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                11/May/20

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 15 minutes
                  15m