Moodle
  1. Moodle
  2. MDL-43905

incorrect code for updates of login times

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.6
    • Fix Version/s: 2.6.2, 2.7
    • Component/s: Libraries
    • Labels:
    • Story Points (Obsolete):
      4
    • Sprint:
      BACKEND Sprint 10

      Description

      during event migration the code updating user login times got borked - he $USER->timemodified and db state get out of sync; it is also painfully slow...

      core_moodlelib_testcase::test_complete_user_login
      Time is lower that allowed start value
      Failed asserting that '1390980862' is equal to 1390980863 or is greater than 1390980863.
      /home/jerome/moodles/integration_master_mysqli/moodle/lib/phpunit/classes/advanced_testcase.php:393
      /home/jerome/moodles/integration_master_mysqli/moodle/lib/tests/moodlelib_test.php:2473
      /home/jerome/moodles/integration_master_mysqli/moodle/lib/phpunit/classes/advanced_testcase.php:80
      

      At the same time this code triggers extra user updated event right before user logged in event which is also not correct.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Petr Skoda added a comment -

            this should fix it:

                user_update_user($user, false);
             
                // Update the timestamp of modification.
                $USER->timemodified = $user->timemodified;
            
            

            Show
            Petr Skoda added a comment - this should fix it: user_update_user($user, false);   // Update the timestamp of modification. $USER->timemodified = $user->timemodified;
            Hide
            Petr Skoda added a comment -

            or just remove the user_update_user() from update_user_login_times() - this would remove the bogus user updated event on each login...

            Show
            Petr Skoda added a comment - or just remove the user_update_user() from update_user_login_times() - this would remove the bogus user updated event on each login...
            Hide
            Eloy Lafuente (stronk7) added a comment -

            +1 to the 2nd solution with update_user_login_times() in charge of updating times by itself, not using user_update_user() anymore.

            Show
            Eloy Lafuente (stronk7) added a comment - +1 to the 2nd solution with update_user_login_times() in charge of updating times by itself, not using user_update_user() anymore.
            Hide
            Rajesh Taneja added a comment -

            Thanks Petr,

            Patch looks spot-on. Pushing it for integration.

            Show
            Rajesh Taneja added a comment - Thanks Petr, Patch looks spot-on. Pushing it for integration.
            Hide
            CiBoT added a comment -

            Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

            Show
            CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
            Hide
            Marina Glancy added a comment -

            Thanks Petr, integrated in 2.6 and master.

            Since it is a change in stable 2.6, do you think we should document/announce this change somewhere?

            Show
            Marina Glancy added a comment - Thanks Petr, integrated in 2.6 and master. Since it is a change in stable 2.6, do you think we should document/announce this change somewhere?
            Hide
            Petr Skoda added a comment -

            we announce changes like this in release page, do we have it already?

            Show
            Petr Skoda added a comment - we announce changes like this in release page, do we have it already?
            Hide
            Marina Glancy added a comment -

            2.6 is already released

            Show
            Marina Glancy added a comment - 2.6 is already released
            Hide
            Petr Skoda added a comment -

            even minor releases have release pages

            Show
            Petr Skoda added a comment - even minor releases have release pages
            Hide
            Marina Glancy added a comment -

            right, marking it as docs_required, needs mentioning on http://docs.moodle.org/dev/Moodle_2.6.2_release_notes

            Show
            Marina Glancy added a comment - right, marking it as docs_required, needs mentioning on http://docs.moodle.org/dev/Moodle_2.6.2_release_notes
            Hide
            Rossiani Wijaya added a comment -

            The unittest is passing as reported from http://integration.moodle.org/.

            Passing this issue.

            Show
            Rossiani Wijaya added a comment - The unittest is passing as reported from http://integration.moodle.org/ . Passing this issue.
            Hide
            Sam Hemelryk added a comment -

            This weeks weekly release is now available and includes your code.
            A big pat on the back to you again for once more being a cog in the Moodle machine.

            Best wishes, the Moodle integration team.

            Show
            Sam Hemelryk added a comment - This weeks weekly release is now available and includes your code. A big pat on the back to you again for once more being a cog in the Moodle machine. Best wishes, the Moodle integration team.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Agile