New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix update, make column nullable #13261
Conversation
core/Updates/3.6.0-b4.php
Outdated
public function getMigrations(Updater $updater) | ||
{ | ||
// use php date since mysql date/time might be different and might lock users out for a while. and subtract days just to be safe. | ||
$passwordModified = Date::factory('now')->subDay(14); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the date-time will be 14 days offset from the actual time where password was changed, would it be possible not to have this 14 days offset somehow? As we may display in the future the time when the password was changed in the UI, the value should be accurate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we shouldn't set the value on Matomo update at all and leave it blank. Guess the other code can be adjusted to ignore the additional checks if the password has never changed...
Setting the value to any other value, like now()
or the user creation date might be wrong when displayed in the UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, will make the column null-able
…d, will not result in an inaccurate display.
@mattab modified the PR & tested on demo2 |
* Do not use mysql time when setting ts password modified initial date. * Correct namespace * Missing import * fix sql * Make ts_password_modified column nullable so when eventually displayed, will not result in an inaccurate display. * fix update
Fixes #13260
CC @mattab