-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Carbon support for time-zone-aware doctrine types #2835
Carbon support for time-zone-aware doctrine types #2835
Comments
As a matter of completeness and consistency, we could add that just like we have for CarbonImmutableType and CarbonType Pull-request would be welcome.
Now as a recommendation: Don't use any kind of entity type mixing both datetime and timezone.You should try to have table columns (and entity property) to represent each piece of the data as atomically as possible. And while having timezone attached to a DateTime/Carbon object is fine and convenient as a high level input received by human, or output shown to them, it's not good to store them like that. They are actually 2 different (and actually unrelated) things: a moment and a place. When receiving from user the result of a datepicker as Then for 99% of the other cases, for instance showing when a message was sent in a messaging application, no timezone at all should be stored, you really must have UTC datetimes here so you can sort/compare consistently those messages and you just need to use the device timezone to show those dates and times. I explained this recommendation more in details in this article Always Use UTC Dates And Times |
Thanks for your response. I agree that dates should always be stored in a way that allows us to sort/compare consistently. It can either be UTC date or date time with time zone. As a general practice, if we do not save time zone in any way, date must be saved in UTC. Though, it is not always convenient to operate with UTC dates on the code level. Therefore, not at all is such swaying time zones back and forth convenient. For aforementioned cases it would be much more easier for us to operate on dates just in the user time zone. And when it comes to saving into DB, we'd save as |
The great advantage of doing this is that the same date can be "worked" on different timezones so to be served to different users having different timezones, or to APIs that would likely be UTC too. So being able to know if a time is I would enjoin you to triple-challenge such seemingly convenient decisions, it's the one kind that might have clear visible benefit and many unnoticeable burden cost to long live with. Saving few calls to That's one of the reason why I will not implement this myself, that too much sounds like a tool that would hurt many users and likely never solve real problems the right way. But this is open to pull-requests. |
I have completed the setup and trying to do this :) |
Do we also need common conversion logic to share between the CarbonTzType and CarbonTzImmutableType classes ? |
Not necessarily, new types should land here: https://github.com/CarbonPHP/carbon-doctrine-types/tree/main/src/Carbon/Doctrine (doctrine types have been externalized in a separate package since this suggestion was open). It contains a trait https://github.com/CarbonPHP/carbon-doctrine-types/blob/main/src/Carbon/Doctrine/CarbonTypeConverter.php that help not having duplicate code across type classes. For instance things such as the maximum precision would be the same for a type with timezone. So we should have write new stuff for that and just re-use the existing trait for everything that works the same. |
Hello,
Doctrine has two types, which handle time-zones:
DateTimeTzImmutableType
andDateTimeTzType
.For carbon specifically, community has already written couple of types, which implement this:
https://github.com/beberlei/DoctrineExtensions/blob/master/src/Types/CarbonImmutableDateTimeTzType.php
https://github.com/beberlei/DoctrineExtensions/blob/master/src/Types/CarbonDateTimeTzType.php
I would like to propose introduction of two doctrine types to handle time zones:
The text was updated successfully, but these errors were encountered: