-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 Scheduled Reports sent one hour late in daylight saving timezones #10443
Conversation
$dateTimeZone = new \DateTimeZone($siteTimezone); | ||
|
||
$view->timeZoneDifference = -$dateTimeZone->getOffset(new \DateTime()); | ||
$view->countWebsites = count(APISitesManager::getInstance()->getSitesIdWithAtLeastViewAccess()); |
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.
Wouldn't this cause a memory hog, if there are lots of sites added?
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.
If so that would have been there before, as well. I've only added the detection for timezone offset.
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.
Ok, I see.
I think it looks OK in general 👍 |
603505e
to
7c6e8c4
Compare
@mkrakiewicz I've updated the code according to your feedback. |
We got a similar report:
Do you think this PR will fix such issue as well, that a report scheduled for 5pm later today in UTC+12 will run today and not tomorrow (in UTC+12 timezone) ? |
Yes, that should fix all time issues regarding scheduled reports. The report hour will always be saved in UTC. And as the scheduling is also done in UTC, the time should always be correct then. |
I'll update the PR later, so it will get mergable again. |
e52f560
to
8b5d83c
Compare
@@ -122,10 +131,12 @@ function initManagePdf() { | |||
|
|||
apiParameters.parameters = getReportParametersFunctions[apiParameters.reportType](); | |||
|
|||
var hour = adjustHourToTimezone($('#report_hour').val()); |
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.
Please provide -timeZoneDifference
as a second argument for adjustHourToTimezone()
.
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.
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.
fixed
8b5d83c
to
f40956a
Compare
f40956a
to
5ef3110
Compare
* Fix Scheduled Reports sent one hour late in daylight saving timezones (#10443) * convert hour to send report to/from UTC, to ensure it isn't affected by daylight savings * adds update script to change existing scheduled reports to use utc time * code improvement * adds missing param * Added new event Archiving.makeNewArchiverObject to allow customising plugin archiving (#10366) * added hook to alllow plugin archiving prevention * cr code style notes * reworked PR to fit CR suggestions * added PHPDoc for hook * Event description more consistent * UI tests: minor changes * Adds test checking if all screenshots are stored in lfs * removed screenshots not stored in lfs * readds screenshots to lfs * 2.16.3-b4
* Fix depraction test: use assertDeprecatedMethodIsRemovedInPiwik3 * Fix Scheduled Reports sent one hour late in daylight saving timezones (#10443) * convert hour to send report to/from UTC, to ensure it isn't affected by daylight savings * adds update script to change existing scheduled reports to use utc time * code improvement * adds missing param * Added new event Archiving.makeNewArchiverObject to allow customising plugin archiving (#10366) * added hook to alllow plugin archiving prevention * cr code style notes * reworked PR to fit CR suggestions * added PHPDoc for hook * Event description more consistent * UI tests: minor changes * Comment out Visitor Log UI tests refs #10536 * Adds test checking if all screenshots are stored in lfs * removed screenshots not stored in lfs * readds screenshots to lfs * 2.16.3-b4 * Issue translation updates against 2.x-dev * language update * Fix bug in widget list remove where the JSON object becomes array * 2.16.3-rc1 * console command custom-piwik-js:update should work when directory is writable and file does not exist yet (#10576) * followup #10449 * Fix test (cherry picked from commit fac3d63) * Prevent chmod(): No such file or directory * Automatically update all marketplace plugins when updating Piwik (#10527) * update plugins and piwik at the same time * make sure plugins are updated with piwik * use only one try/catch * reload plugin information once it has been installed * make sure to clear caches after an update * fix ui tests * make sure to use correct php version without any extras * Additional informations passed in the hook "isExcludedVisit" (issue #10415) (#10564) * Additional informations passed in the hook "isExcludedVisit" (issue #10415) * Added better description to the new parameters * Update VisitExcluded.php * Remove two parameters not needed as better to use the Request object * Update VisitExcluded.php * remove extra two parameters in VisitExcluded constructor to prevent confusion (#10593) * Update our libs to latest #10526 * Update composer libraries to latest #10526 * Update log analytics to latest * When updating the config file failed (or when any other file is not writable...), the Updater (for core or plugins) will now automatically throw an error and cancel the update (#10423) * When updating the config file failed (or when any other file is not writable...), the Updater (for core or plugins) will now automatically throw an error and cancel the update * add integration test to check the correct exception is thrown when config not writabel * New integration test for updater * Make test better * When opening the visitor profile, do not apply the segment (#10533) * When opening the visitor profile, do not apply the segment * added ui test for profile but does work for me * next try to make ui test work * add expected screenshot * added missing doc
[Replaces #10318]
This PR aims to fix the issue, that scheduled reports might be sent one hour to late / to early if the reports are saved for an site having a timezone using daylight savings.
The problem occurs because the "hour" the report should be sent at is currently saved in the timezone of the site. That means a new report with the timezone like e.g.
Europe/Berlin
with the setting to send the report at 8 o'clock would be one hour earlier/later when the daylight saving changes.Internal Piwik uses UTC for running the tasks, so it tries to adjust the saved hour using the site's timezone difference. At the moment that would be two hours for
Europe/Berlin
, so the report would sent at 6 o'clock UTC. When the daylight saving changes again, the timezone difference will be only an hour. So Piwik will then reduce the saved hour value only by one to get the UTC time, which will be 7 o'clock (which is one hour later).To avoid this in the future, I changed the current behavior to not save the hour value in the site's timezone anymore, but to use the UTC time instead. Doing this, the value can't be affected by daylight saving anymore. I also added an update script to update existing reports to have UTC values.
When creating/editing an report for an site using a timezone with a difference to UTC, Piwik will now should the UTC, aswell:
fixes #7658, refs #5873