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

Assignment upgrade tool overwrites unrelated records in gradebook

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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:

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            damyon Damyon Wiese added a comment -

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

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

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

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

            Thanks Ankit. Submitting now.

            Show
            damyon Damyon Wiese added a comment - Thanks Ankit. Submitting now.
            Hide
            poltawski 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
            poltawski 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
            poltawski Dan Poltawski added a comment -

            I've integrated this now

            Show
            poltawski Dan Poltawski added a comment - I've integrated this now
            Hide
            damyon 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 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
            poltawski 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
            poltawski 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 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 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
            rajeshtaneja Rajesh Taneja added a comment -

            Works as expected.
            itemname is db is correctly set.

            Thanks for fixing this Damyon

            Show
            rajeshtaneja Rajesh Taneja added a comment - Works as expected. itemname is db is correctly set. Thanks for fixing this Damyon
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  25/Jun/12