[DoctrineBridge] Add new DayPointType and TimePointType Doctrine type
#60237
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
|
How is it "problematic"? |
|
@garak For example, Doctrine Migrations for PostgreSQL will generate migrations using the TIMESTAMP type instead of DATE. The schema will never be fully synchronized if you try to force it. Relational databases have dedicated data types for storing DATE and DATETIME, which is why PHP's DateTime is represented by two separate types in Doctrine. |
|
First of all, thank you for bringing up this topic. Secondly: Naming things is hard.
My proposal: Keep the |
|
@derrabus Ok, so we're leaning more towards Symfony's naming conventions rather than Doctrine's. You contribute a lot to Doctrine, so this is an important note. |
|
@derrabus After the changes you suggested, this is how it will look. doctrine:
dbal:
types:
date_point: Symfony\Bridge\Doctrine\Types\DatePointType
date_point_date: Symfony\Bridge\Doctrine\Types\DatePointDateType#[ORM\Column(type: 'date_point_date')]
public DatePoint $birthday;
#[ORM\Column(type: 'date_point')]
public DatePoint $createdAt; |
acb035e to
7d6da09
Compare
7d6da09 to
31ca901
Compare
DatePointDateType Doctrine type
e5ec79a to
9f41419
Compare
|
Added missing test |
9f41419 to
c18da98
Compare
|
Updated Changelog 7.3 ->7.4. |
|
@carsonbot label Feature |
|
Just an idea: What about keeping the name add make it configurable instead of having two types? |
…Type` and `TimeType` (wkania) This PR was merged into the 7.4 branch. Discussion ---------- [Form] Add `input=date_point` to `DateTimeType`, `DateType` and `TimeType` | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | | License | MIT Based on [datetime_immutable](https://symfony.com/blog/new-in-symfony-4-1-added-support-for-immutable-dates-in-forms). After [DatePointType](#59900) and [DatePointDateType](#60237), it would be great to use Forms without needing to transform values into the DatePoint type manually. ``` use Symfony\Component\Form\Extension\Core\Type\DateType; use Symfony\Component\Form\Extension\Core\Type\DateTimeType; use Symfony\Component\Form\Extension\Core\Type\TimeType; use Symfony\Component\Form\Extension\Core\Type\BirthdayType; $builder->add('from', DateType::class, [ 'input' => 'date_point', ]); $builder->add('from', DateTimeType::class, [ 'input' => 'date_point', ]); $builder->add('from', TimeType::class, [ 'input' => 'date_point', ]); $builder->add('from', BirthdayType::class, [ 'input' => 'date_point', ]); ``` Alternative: Make symfony/clock a hard requirement and refactor the existing DateTimeImmutableToDateTimeTransformer to return a DatePoint instead. This should not introduce any breaking changes. Commits ------- f1160d6 [Form] Add input=date_point to DateTimeType, DateType and TimeType
…Type` and `TimeType` (wkania) This PR was merged into the 7.4 branch. Discussion ---------- [Form] Add `input=date_point` to `DateTimeType`, `DateType` and `TimeType` | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | | License | MIT Based on [datetime_immutable](https://symfony.com/blog/new-in-symfony-4-1-added-support-for-immutable-dates-in-forms). After [DatePointType](symfony/symfony#59900) and [DatePointDateType](symfony/symfony#60237), it would be great to use Forms without needing to transform values into the DatePoint type manually. ``` use Symfony\Component\Form\Extension\Core\Type\DateType; use Symfony\Component\Form\Extension\Core\Type\DateTimeType; use Symfony\Component\Form\Extension\Core\Type\TimeType; use Symfony\Component\Form\Extension\Core\Type\BirthdayType; $builder->add('from', DateType::class, [ 'input' => 'date_point', ]); $builder->add('from', DateTimeType::class, [ 'input' => 'date_point', ]); $builder->add('from', TimeType::class, [ 'input' => 'date_point', ]); $builder->add('from', BirthdayType::class, [ 'input' => 'date_point', ]); ``` Alternative: Make symfony/clock a hard requirement and refactor the existing DateTimeImmutableToDateTimeTransformer to return a DatePoint instead. This should not introduce any breaking changes. Commits ------- f1160d6617f [Form] Add input=date_point to DateTimeType, DateType and TimeType
|
@OskarStark
|
There was a problem hiding this comment.
Naming :)
what about DayPointType (and TimePointType)?
can you please rebase?
Configurable types are hard to achieve in DBAL (and supported properly only in DBAL 4 IIRC) |
DayPoint sounds better. So we will have: I will rebase, rename and add TimePointType later today. |
c18da98 to
f4d9b0d
Compare
DatePointDateType Doctrine typeDayPointType and TimePointType Doctrine type
f4d9b0d to
38b6d6e
Compare
|
Rebased, renamed and added TimePointType. Falling tests not related. |
src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterDatePointTypePass.php
Outdated
Show resolved
Hide resolved
38b6d6e to
7f383ef
Compare
|
Thank you @wkania. |
Doctrine provides both date_immutable and datetime_immutable types. Restricting the conversion of DatePoint only to datetime_immutable is problematic.
New version:
Previous pull request.
Old version:
Therefore, I propose renaming the current DatePointType to DateTimePointType, and introducing a new DatePointType.
Previous pull request.
If this pull request is to be merged, it should be included in version 7.3 to avoid any breaking changes in the future.