Moodle
  1. Moodle
  2. MDL-33793

Assignment upgrade tool overwrites unrelated records in gradebook

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Assignment
    • Labels:
    • Testing Instructions:
      Hide
      1. Perform a clean install of Moodle to a fresh database.
      2. Enable the Assignment 2.2 module
      3. Create a new course (does not require users)
      4. Add 1 quiz to the course (Just name it and leave everything at defaults)
      5. Add 1 assignment 2.2 online text to the course
      6. After this setup you should have 3 entries in the grade_items table all with iteminstance set to 1, the itemmodule column should be NULL, quiz and assignment.
      7. Upgrade the assignment instance
      8. After the upgrade the grade_item table should have 3 entries and the itemmodule column should be NULL, quiz and assign (without this fix, the itemmodule column for all three entries are updated to assign)
      Show
      Perform a clean install of Moodle to a fresh database. Enable the Assignment 2.2 module Create a new course (does not require users) Add 1 quiz to the course (Just name it and leave everything at defaults) Add 1 assignment 2.2 online text to the course After this setup you should have 3 entries in the grade_items table all with iteminstance set to 1, the itemmodule column should be NULL, quiz and assignment. Upgrade the assignment instance After the upgrade the grade_item table should have 3 entries and the itemmodule column should be NULL, quiz and assign (without this fix, the itemmodule column for all three entries are updated to assign)
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
    • Rank:
      41847

      Description

      Reported by Michael Woods on MDL-33464.

      Assignment upgrade tool needs additional where clauses so it does not affect unrelated grade items. This is a serious bug and silently damages the data in the gradebook for anyone using the assignment upgrade tool.

        Issue Links

          Activity

          Hide
          Damyon Wiese added a comment -

          This is a second issue that was reported as part of MDL-33464.

          Show
          Damyon Wiese added a comment - This is a second issue that was reported as part of MDL-33464 .
          Hide
          Ankit Agarwal added a comment -

          Hi Damyon,
          This looks good.
          Feel free to submit for integration.
          Thanks

          Show
          Ankit Agarwal added a comment - Hi Damyon, This looks good. Feel free to submit for integration. Thanks
          Hide
          Damyon Wiese added a comment -

          Thanks Ankit. Submitting now.

          Show
          Damyon Wiese added a comment - Thanks Ankit. Submitting now.
          Hide
          Dan Poltawski added a comment -

          Wow, this is a very scary issue to see come through.

          I think we need some detailed code review on upgradelib.php in order to make sure there are no other problems around.

          Show
          Dan Poltawski added a comment - Wow, this is a very scary issue to see come through. I think we need some detailed code review on upgradelib.php in order to make sure there are no other problems around.
          Hide
          Dan Poltawski added a comment -

          I've integrated this now

          Show
          Dan Poltawski added a comment - I've integrated this now
          Hide
          Damyon Wiese added a comment -

          Thanks, any additional eyes on upgradelib would be appreciated as this was difficult code to write given that most of the upgrading had to be done with direct DB queries on tables outside of the assignment/assign modules. This bug was hard to spot as it affected other modules (ie it would never break mod_assign - just random other modules). Big thanks to Michael Woods for reporting it and following up on the linked ticket.

          Show
          Damyon Wiese added a comment - Thanks, any additional eyes on upgradelib would be appreciated as this was difficult code to write given that most of the upgrading had to be done with direct DB queries on tables outside of the assignment/assign modules. This bug was hard to spot as it affected other modules (ie it would never break mod_assign - just random other modules). Big thanks to Michael Woods for reporting it and following up on the linked ticket.
          Hide
          Dan Poltawski added a comment -

          I've asked Fred to take a look at this code for me. It'd be good to know what other DB tables we are affecting other than mod_assign tables to double check there.

          And agreed - many thanks Michael for spotting this.

          Show
          Dan Poltawski added a comment - I've asked Fred to take a look at this code for me. It'd be good to know what other DB tables we are affecting other than mod_assign tables to double check there. And agreed - many thanks Michael for spotting this.
          Hide
          Damyon Wiese added a comment -

          Thanks Dan,

          The other tables affected by the upgrade are:

          modules, course_sections, events, grade_items, course_modules_completion, course_completion_criteria, grading_instances and grading_areas.

          Show
          Damyon Wiese added a comment - Thanks Dan, The other tables affected by the upgrade are: modules, course_sections, events, grade_items, course_modules_completion, course_completion_criteria, grading_instances and grading_areas.
          Hide
          Rajesh Taneja added a comment -

          Works as expected.
          itemname is db is correctly set.

          Thanks for fixing this Damyon

          Show
          Rajesh Taneja added a comment - Works as expected. itemname is db is correctly set. Thanks for fixing this Damyon
          Hide
          Eloy Lafuente (stronk7) added a comment -

          And this has been spread to every git and cvs repository out there, just in time to roll Moodle 2.3beta!

          Thanks! Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - And this has been spread to every git and cvs repository out there, just in time to roll Moodle 2.3beta! Thanks! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: