If recurring periods are to properly support timezones then the recurring period plugin must know what timezone it is dealing with. #2912347: Use \DateTimeImmutable rather than timestamp for all time calculations adds support for this by using DateTimeImmutable objects instead of timestamps. That change would make the caller of the plugin (in this case commerce_licence) responsible for supplying the correct timezone.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 2914350-4-commerce-licence-timezones.patch | 6.16 KB | erik.erskine |
Comments
Comment #2
erik.erskine commentedThis patch updates commerce licence to reflect the API change in #2912347: Use \DateTimeImmutable rather than timestamp for all time calculations and use a timezone appropriate to the licence owner. It modifies the
Licence::calculateExpirationTimefunction to pass a timezone to the recurring period plugin. Both start and expiry values are still persisted as UTC timestamps.That's a start, but it's possible that this behaviour is desired for some licences, whereas others should be based on the site timezone, UTC, or something else. Do we need some kind of selection for that, in a similar way to how date fields are handled?
Comment #4
erik.erskine commentedUpdated #2 to fix tests, with the following changes:
calculateExpirationTimenow returnsNULLrather than0if there is no expiration dateComment #5
erik.erskine commentedComment #8
joachim commentedCommitted with a few tweaks:
- I don't think this issue should be changing the value we store for an unlimited license
- fixes to docs formatting
- tweak to tests -- we don't need to bother with user storage, just use createUser()
- not sure why there was a (string) cast in commerce_licence_get_user_timezone(). Generally, it's a real shame we have to have this -- core should provide something like this!