Skip to content

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Jul 26, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Currently, when reference_date and model_timezone are provided but are not the same, we get a validation exception:

if (null !== $options['reference_date'] && $options['reference_date']->getTimezone()->getName() !== $options['model_timezone']) {
throw new InvalidConfigurationException(sprintf('The configured "model_timezone" (%s) must match the timezone of the "reference_date" (%s).', $options['model_timezone'], $options['reference_date']->getTimezone()->getName()));
}

But that happen even if the model_timezone is null !

I propose to relax this behavior by passing the time zone of the configured reference_date to this model_timezone (by default), thus avoiding annoyances.

@yceruto yceruto added this to the next milestone Jul 26, 2019
@carsonbot carsonbot added Status: Needs Review Form DX DX = Developer eXperience (anything that improves the experience of using Symfony) Feature labels Jul 26, 2019
@xabbuh
Copy link
Member

xabbuh commented Jul 26, 2019

This looks good. Can you also add a test for this?

@yceruto
Copy link
Member Author

yceruto commented Jul 28, 2019

Test added.

@xabbuh
Copy link
Member

xabbuh commented Jul 29, 2019

Thank you @yceruto.

@xabbuh xabbuh merged commit a62feea into symfony:4.4 Jul 29, 2019
xabbuh added a commit that referenced this pull request Jul 29, 2019
… option when possible (yceruto)

This PR was merged into the 4.4 branch.

Discussion
----------

[Form][DX] derive default timezone from reference_date option when possible

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Currently, when `reference_date` and `model_timezone` are provided but are not the same, we get a validation exception:
https://github.com/symfony/symfony/blob/9216cb75ac11db014262f2d7fe5cb77b0b54db59/src/Symfony/Component/Form/Extension/Core/Type/TimeType.php#L48-L50

But that happen even if the `model_timezone` is `null` !

I propose to relax this behavior by passing the time zone of the configured `reference_date` to this `model_timezone` (by default), thus avoiding annoyances.

Commits
-------

a62feea [DX] derive default timezone from reference_date option when possible
@yceruto yceruto deleted the time_type branch July 29, 2019 15:54
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony) Feature Form Status: Reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants