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

The Mooodle library lib/moodlelib.php does not update the property timemodified on deletion methods, to pass to the deleted events.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.2
    • Fix Version/s: 2.2.2
    • Component/s: Libraries
    • Labels:

      Description

      The Mooodle library lib/moodlelib.php does not update the user->timemodified nor the course->timemodified on deletion methods, to pass to the user_deleted and course_deleted events.

      The user record gets the property timemodified updated when set to deleted, but it is not passed to the user_deleted event, which would register inconsistent values.
      Analogously, the course_deleted event should update the timemodified property before triggering the event. The course is in fact removed from the database, but the course deleted event may require to know the real time that happened.

      Patch included.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            aborrow Anthony Borrow added a comment -

            I've moved this from CONTRIB to MDL and am re-assigning to moodle.com so it can be triaged. Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - I've moved this from CONTRIB to MDL and am re-assigning to moodle.com so it can be triaged. Peace - Anthony
            Hide
            aborrow Anthony Borrow added a comment -

            Luis - Thanks for filing the bug report and patch. I've moved it to the Moodle project and assigned it to be evaluated by moodle.com. Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - Luis - Thanks for filing the bug report and patch. I've moved it to the Moodle project and assigned it to be evaluated by moodle.com. Peace - Anthony
            Hide
            luis.alcantara Luis Gustavo Mueller de Alcantara added a comment -

            Thanks Anthony for moving this to MDL, and I'm sorry about my mistake.

            Show
            luis.alcantara Luis Gustavo Mueller de Alcantara added a comment - Thanks Anthony for moving this to MDL, and I'm sorry about my mistake.
            Hide
            aborrow Anthony Borrow added a comment -

            No worries at all Luis. Thanks for the report and thanks to Petr for picking it up so quickly. Providing a patch is a great help too. Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - No worries at all Luis. Thanks for the report and thanks to Petr for picking it up so quickly. Providing a patch is a great help too. Peace - Anthony
            Hide
            skodak Petr Skoda added a comment -

            I agree that external systems informed via events may want to know when the user or course was deleted.

            To integrators: This may be seen as a change of current events API, please consider this when deciding if this should be backported to older stable branches. My +1 for 2.2 only

            Show
            skodak Petr Skoda added a comment - I agree that external systems informed via events may want to know when the user or course was deleted. To integrators: This may be seen as a change of current events API, please consider this when deciding if this should be backported to older stable branches. My +1 for 2.2 only
            Hide
            luis.alcantara Luis Gustavo Mueller de Alcantara added a comment -

            Just to clarify:

            When considering if this should be backported to older stable branches, the integrators should know that there could be found registry of users with the same timecreated and timemodified timestamps, which would be really dificult to get on ordinary Moodle use. (Not under the Moodle user table, but on external registry, done throught events.)

            I hope it helps Moodle Administrators on understanding what happened if those cases are found.

            Thanks for the help Petr.

            Show
            luis.alcantara Luis Gustavo Mueller de Alcantara added a comment - Just to clarify: When considering if this should be backported to older stable branches, the integrators should know that there could be found registry of users with the same timecreated and timemodified timestamps, which would be really dificult to get on ordinary Moodle use. (Not under the Moodle user table, but on external registry, done throught events.) I hope it helps Moodle Administrators on understanding what happened if those cases are found. Thanks for the help Petr.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

            This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

            This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

            Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) 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
            stronk7 Eloy Lafuente (stronk7) 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
            samhemelryk Sam Hemelryk added a comment -

            Thanks guys, this has been integrated now on master and MOODLE_22_STABLE. I've decided not to backport this to 2.1 sorry.
            Please note that I had to make an additional commit to clean up whitespace, please watch that.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks guys, this has been integrated now on master and MOODLE_22_STABLE. I've decided not to backport this to 2.1 sorry. Please note that I had to make an additional commit to clean up whitespace, please watch that. Cheers Sam
            Hide
            phalacee Jason Fowler added a comment -

            Petr, could you please provide better testing instructions?

            Show
            phalacee Jason Fowler added a comment - Petr, could you please provide better testing instructions?
            Hide
            rwijaya Rossiani Wijaya added a comment -

            This looks good.

            test passed.

            Show
            rwijaya Rossiani Wijaya added a comment - This looks good. test passed.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            This virus has been spread upstream, everybody will be infected soon. Congrats, you did it!

            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - This virus has been spread upstream, everybody will be infected soon. Congrats, you did it! Closing, ciao
            Hide
            luis.alcantara Luis Gustavo Mueller de Alcantara added a comment -

            Thanks for the update, and for keeping things clear about affected versions.

            Congrats.

            Show
            luis.alcantara Luis Gustavo Mueller de Alcantara added a comment - Thanks for the update, and for keeping things clear about affected versions. Congrats.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Mar/12