Skip to content
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

Fix #19531 [Form] DateType fails parsing when midnight is not a valid time #19541

Merged
merged 1 commit into from Aug 13, 2016

Conversation

Projects
None yet
6 participants
@mbeccati
Copy link
Contributor

commented Aug 5, 2016

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

@mbeccati mbeccati force-pushed the mbeccati:19531-datetype-fix-27 branch from 4153a78 to 45430c6 Aug 5, 2016

@Jean85

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2016

Thanks for fixing this so fast!

Status: reviewed

*/
protected function isPatternDateOnly()
{
// the default pattern includes time

This comment has been minimized.

Copy link
@javiereguiluz

javiereguiluz Aug 5, 2016

Member

Maybe this comment is unnecessary?

return false;
}
// strip escaped text

This comment has been minimized.

Copy link
@javiereguiluz

javiereguiluz Aug 5, 2016

Member

And this comment may be unnecessary too?

This comment has been minimized.

Copy link
@mbeccati

mbeccati Aug 5, 2016

Author Contributor

I don't know - ICU escaping is not very common and it might not be clear what we're doing here.

} catch (\Exception $e) {
throw new TransformationFailedException($e->getMessage(), $e->getCode(), $e);
}
if ('UTC' !== $this->inputTimezone) {
// perform the conversion only when necessary

This comment has been minimized.

Copy link
@javiereguiluz

javiereguiluz Aug 5, 2016

Member

Maybe this comment is unnecessary?

@mbeccati mbeccati force-pushed the mbeccati:19531-datetype-fix-27 branch from 45430c6 to c951bb6 Aug 5, 2016

@mbeccati

This comment has been minimized.

Copy link
Contributor Author

commented Aug 5, 2016

@javiereguiluz Pushed comment amendments + additional test w/ escaped text

}
// strip escaped text
$pattern = preg_replace("#'(.*?)'#", '', $this->pattern);

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Aug 9, 2016

Member

Can you add a test with escaped ICU data?

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Aug 9, 2016

Member

oh, it's there, I had to right scroll :)

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Aug 9, 2016

👍

@fabpot

This comment has been minimized.

Copy link
Member

commented Aug 13, 2016

Thank you @mbeccati.

@fabpot fabpot merged commit c951bb6 into symfony:2.7 Aug 13, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Aug 13, 2016

bug #19541 Fix #19531 [Form] DateType fails parsing when midnight is …
…not a valid time (mbeccati)

This PR was merged into the 2.7 branch.

Discussion
----------

Fix #19531 [Form] DateType fails parsing when midnight is not a valid time

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

Commits
-------

c951bb6 Fix #19531 [Form] DateType fails parsing when midnight is not a valid time

This was referenced Sep 2, 2016

mbeccati added a commit to mbeccati/symfony that referenced this pull request Sep 7, 2016

Fix symfony#19721
Issue was introduced in symfony#19541

mbeccati added a commit to mbeccati/symfony that referenced this pull request Sep 7, 2016

Fix symfony#19721
Issue was introduced in symfony#19541

fabpot added a commit that referenced this pull request Sep 8, 2016

bug #19879 [Form] Incorrect timezone with DateTimeLocalizedStringTran…
…sformer (mbeccati)

This PR was merged into the 2.7 branch.

Discussion
----------

[Form] Incorrect timezone with DateTimeLocalizedStringTransformer

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #19721
| License       | MIT
| Doc PR        | n.a.

Issue was introduced in #19541

Commits
-------

bf6691c Fix #19721

@mbeccati mbeccati deleted the mbeccati:19531-datetype-fix-27 branch Sep 9, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.