[Validator] Fixed time constraint (support formats H, H:i and H:i:s) #9163
Conversation
…y, with minutes and with minutes and seconds | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | -
…ith minutes and with minutes and seconds | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | -
| { | ||
| parent::__construct($options); | ||
|
|
||
| if (isset($options['withMinutes']) && !isset($options['withSeconds']) && $options['withMinutes'] == false) { |
There was a problem hiding this comment.
if (isset($options['with_minutes']) && !isset($options['with_minutes']) && !$options['with_minutes']) { There was a problem hiding this comment.
@stloyd Correct! But if we do so then we must also rename the properties e.g. protected $with_minutes; and that's not want we want to do.
We need something that converts option names to camelcased strings. How is this handled in other components?
There was a problem hiding this comment.
In overall php variables are camelCased, while array keys are written with underscore.
There was a problem hiding this comment.
take a look into the base constraint class how options are set:
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/Constraint.php#L98
if you pass an option named "with_minutes" it's value is not set for the property "withMinutes".
There was a problem hiding this comment.
In https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/Constraints/Length.php
The options are first passed to the parents constructor, and then the properties are checked.
There was a problem hiding this comment.
Camel casing is fine here.
There was a problem hiding this comment.
it should be === false rather than == false
There was a problem hiding this comment.
my own comment is not fixed though
|
👍 |
|
ping @symfony/deciders |
|
The support for |
|
hmm, this is because the PR is not created from your fork anymore as it is a new one. However, there is a way to create a local branch based on the branch you use for this PR, allowing you to work on it again and to create a new PR with it: git fetch https://github.com/symfony/symfony.git refs/pull/9163
git branch time_constraint_format FETCH_HEAD
git checkout time_constraint_format |
|
Closing in favor of #11821 |
Time constraint should support values with hours only, hours with minutes and hours with minutes and seconds