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

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

          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