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 Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.2
    • Fix Version/s: 2.2.2
    • Component/s: Libraries
    • Labels:
    • Rank:
      33936

      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.

      1. patchmoodlelib.diff
        1 kB
        Luis Gustavo Mueller de Alcantara

        Activity

        Hide
        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
        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
        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
        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 Gustavo Mueller de Alcantara added a comment -

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

        Show
        Luis Gustavo Mueller de Alcantara added a comment - Thanks Anthony for moving this to MDL, and I'm sorry about my mistake.
        Hide
        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
        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
        Petr Škoda 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
        Petr Škoda 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 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 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
        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
        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
        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
        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
        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
        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
        Jason Fowler added a comment -

        Petr, could you please provide better testing instructions?

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

        This looks good.

        test passed.

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

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

        Closing, ciao

        Show
        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 Gustavo Mueller de Alcantara added a comment -

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

        Congrats.

        Show
        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: