[Form] reverse transform RFC 3339 formatted dates #28712
Conversation
| @@ -81,7 +81,7 @@ public function reverseTransform($dateTimeLocal) | |||
| return; | |||
| } | |||
|
|
|||
| if (!preg_match('/^(\d{4})-(\d{2})-(\d{2})[T ]\d{2}:\d{2}(?::\d{2})?$/', $dateTimeLocal, $matches)) { | |||
| if (!preg_match('/^(\d{4})-(\d{2})-(\d{2})[T ]\d{2}:\d{2}(?::\d{2})?(?:\.\d)?(?:Z|(?:(?:\+|-)\d{2}:\d{2}))?$/', $dateTimeLocal, $matches)) { | |||
There was a problem hiding this comment.
Maybe it would be possible to also support timezone without ":"?
if (!preg_match('/^(\d{4})-(\d{2})-(\d{2})[T ]\d{2}:\d{2}(?::\d{2})?(?:\.\d)?(?:Z|(?:(?:\+|-)\d{2}:\d{2})|(?:\+|-)\d{4})?$/', $dateTimeLocal, $matches)) {
There was a problem hiding this comment.
Can you give an example string of what you have in mind so I can add it to the tests?
There was a problem hiding this comment.
done, thank you for the feedback
Technically, dates formatted according to the HTML specifications do not contain any timezone information. But since our DateTimeType used to contain this information in the passed, users had configure their JS libraries to accept (and create) dates in that format. To not break BC we should accept these dates and silently ignore the additional timezone information.
36f9c8f to
503239f
Compare
|
+1 for being clearer about what should work and not to be able to be stricter in the future |
|
@HeahDude Yeah, I am already preparing some PRs to improve DX around this. |
|
@xabbuh Do you already know if this will be accepted? Because if it is not we have to fix our JS, otherwise we would only have to add this version as conflict, and we could release a new working version immediately. |
|
@danrot Adding the conflict rule will be enough. |
|
I've added the conflict for symfony 3.4.16 and 3.4.17 in our composer.json, but if you don't set our latest version (1.6.22) as minimum in the project composer.json, composer will install Sulu 1.6.21, so that it can use Symfony 3.4.17, which then still breaks the installation (hope you understand what I mean, otherwise I can elaborate further 🙈). So people are really struggling with the installation of our product... Is there any other thing we can do, apart from kindly asking you to get this merged and release it? 😊 |
Huh? I have |
|
Just tested it again, had a project with - "sulu/sulu": "~1.6.22",
+ "sulu/sulu": "~1.6.0",Then I executed another In addition to that composer even tells me that Symfony is the reason for not installing 1.6.22: Just realized that it works, if the |
Ah! See, we have |
|
And since |
|
Anyhow, please somebody merge this to the LTS branches 2.8 and 3.4 so we can safely rely on backwards compatibility again. 🙂 |
|
Thank you @xabbuh. |
This PR was merged into the 2.8 branch. Discussion ---------- [Form] reverse transform RFC 3339 formatted dates | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28699 | License | MIT | Doc PR | Technically, dates formatted according to the HTML specifications do not contain any timezone information. But since our DateTimeType used to contain this information in the passed, users had configure their JS libraries to accept (and create) dates in that format. To not break BC we should accept these dates and silently ignore the additional timezone information. Commits ------- 503239f reverse transform RFC 3339 formatted dates
Technically, dates formatted according to the HTML specifications do not
contain any timezone information. But since our DateTimeType used to
contain this information in the passed, users had configure their JS
libraries to accept (and create) dates in that format.
To not break BC we should accept these dates and silently ignore the
additional timezone information.