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

          Attachments

            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