[Form][DateTimeToArrayTransformer] Check for hour, minute & second validity #9441
Conversation
|
I can't find a way to fix failing tests after applying this patch. Can you explain the use case of the failing tests which result of this PR? |
| @@ -160,6 +160,18 @@ public function reverseTransform($value) | |||
| throw new TransformationFailedException('This is an invalid date'); | |||
| } | |||
|
|
|||
| if (isset($value['hour']) && !ctype_digit($value['hour']) && !is_int($value['hour'])) { | |||
There was a problem hiding this comment.
You can simplify this and the following lines to
if (isset($value['hour']) && !ctype_digit((string) $value['hour'])) {There was a problem hiding this comment.
I have based this condition according to the other related to the date field (just above this change). Should I change them too?
There was a problem hiding this comment.
For the record, I have changed them.
|
The failing tests in AbstractLayoutTest were intended to test the output escaping of the values. Now that you added strict validation of the values, you can change the test to pass numbers instead of strings. |
|
@bschussek Okay thx for your feedback! |
|
IMO, the PR is ready! |
|
@bschussek @fabpot Something else needs to be done? |
|
ping someone 👶 :) |
… second validity (egeloen) This PR was submitted for the 2.3-dev branch but it was merged into the 2.3 branch instead (closes #9441). Discussion ---------- [Form][DateTimeToArrayTransformer] Check for hour, minute & second validity | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #9440 | License | MIT | Doc PR | - This PR checks if hour, minute & second values are valid in the datetime to array transformer (values must be integer if they exist). Commits ------- 1543653 [Form][DateTimeToArrayTransformer] Check for hour, minute & second validity
This PR checks if hour, minute & second values are valid in the datetime to array transformer (values must be integer if they exist).