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

incorrect code for updates of login times

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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
            skodak Petr Skoda added a comment -

            this should fix it:

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

            Show
            skodak Petr Skoda added a comment - this should fix it: user_update_user($user, false);   // Update the timestamp of modification. $USER->timemodified = $user->timemodified;  
            Hide
            skodak 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
            skodak 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
            stronk7 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
            stronk7 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
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Petr,

            Patch looks spot-on. Pushing it for integration.

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

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

            Show
            cibot CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
            Hide
            marina 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 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
            skodak Petr Skoda added a comment -

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

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

            2.6 is already released

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

            even minor releases have release pages

            Show
            skodak Petr Skoda added a comment - even minor releases have release pages
            Hide
            marina 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 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
            rwijaya Rossiani Wijaya added a comment -

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

            Passing this issue.

            Show
            rwijaya Rossiani Wijaya added a comment - The unittest is passing as reported from http://integration.moodle.org/ . Passing this issue.
            Hide
            samhemelryk 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
            samhemelryk 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:
                  Fix Release Date:
                  10/Mar/14

                  Agile