@bx80
Description:
Fixes #18773
When archiving a week, month or year some sub-periods may fall in the future (eg. it's Thursday so we want to archive this week, but Friday, Saturday and Sunday are in the future).
Writing blank archive data for the future causes problems (and is inefficient) so the code in Archive.php#L629-L643 skips any archives for periods with a start date in the future or before the website was created. However for some reason it only skips archives with start dates two days greater than the current date, this causes blank archives to be written for today+1 and today+2.
This is a simple fix to skip any periods which start on a date later than the current date.
Prior to this change a running ./console core:archive command would result in Archive::prepareArchive() being called for dates two days into the future, after the change the method is only called up to and include the current date.
Review
- [ ] Functional review done
- [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
- [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
- [ ] Security review done
- [ ] Wording review done
- [ ] Code review done
- [ ] Tests were added if useful/possible
- [ ] Reviewed for breaking changes
- [ ] Developer changelog updated if needed
- [ ] Documentation added if needed
- [ ] Existing documentation updated if needed
@sgiehl
@bx80 could you maybe add some tests for that? Could be a simple test that triggers the archiving and checks which records are created in the archive tables.
Note: To ensure the test assumes a certain date is in the future, you can manipulate the "current time" by setting Date::$now
The code looks fine. But what I'm not fully sure about is, if that might cause issues when using other timezones. But guess even if the website is in a timezone that is ahead UTC, the start date should still not be in the future. 🤔 🤯
@tsteur would you mind having a quick look at this one. I guess it should be fine. But I'm always unsure if those changes could result in any timezone related issues. Maybe we should check if we have enough tests that are using other timezones to ensure this won't cause any problems?
Ok. Guess we should then start adding some archiving tests for sites with timezones ahead of UTC, while faking a specific time to ensure the timezone is already a day ahead.
I've added an additional test which will expect the current local date to be included for archiving even if it's ahead of UTC. The range in the future check is now based of the site local time date rather than the UTC date at midnight.
@bx80 guess that logically makes sense. Would you mind also adding a test case where the site's timezone is behind UTC and UTC time is already a day ahead? Guess in that case it shouldn't create archives as well, right?