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

[Form] Support \DateTimeImmutable #25582

Closed
wants to merge 7 commits into from

Conversation

vudaltsov
Copy link
Contributor

@vudaltsov vudaltsov commented Dec 22, 2017

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #9508
License MIT
Doc PR symfony/symfony-docs#8920

This PR implements input=datetime_immutable. Replaces #25273.

throw new TransformationFailedException('Expected a \DateTimeImmutable.');
}

return new \DateTime($value->format(\DateTime::RFC3339));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use createFromFormat here to remove the need for PHP to guess the date format.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jvasseur , good idea, thanx.

@nicolas-grekas nicolas-grekas added this to the 4.1 milestone Dec 22, 2017
*
* @throws TransformationFailedException If the given value is not a \DateTimeImmutable
*/
public function transform($value): ?\DateTime
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the return type hint breaks the prototype.

*
* @throws TransformationFailedException If the given value is not a \DateTime
*/
public function reverseTransform($value): ?\DateTimeImmutable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the return type hint breaks the prototype.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you, fixed!

@nicolas-grekas
Copy link
Member

Status: needs work

@vudaltsov
Copy link
Contributor Author

Status: Needs Review

@vudaltsov
Copy link
Contributor Author

Anything else needed here?

Copy link
Contributor

@HeahDude HeahDude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice approach to leverage the feature without too much overhead, until we make the required change to make it the default norm format.
LGTM

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(small rebase needed)

@vudaltsov
Copy link
Contributor Author

Rebased. Added a changelog entry and replaced previously added assertDateTimeImmutableEquals with assertEqual.

If everything's good after this, I will squash commits.

@fabpot
Copy link
Member

fabpot commented Feb 7, 2018

Thank you @vudaltsov.

@fabpot fabpot closed this in ec89ac8 Feb 7, 2018
@ste93cry
Copy link
Contributor

ste93cry commented Feb 9, 2018

Are there plans to port this to at least Symfony 3.4?

@derrabus
Copy link
Member

derrabus commented Feb 9, 2018

@ste93cry Since this is considered to be a new feature, it most likely won't land in 3.4, which is in bugfix-only mode.

@vudaltsov
Copy link
Contributor Author

@ste93cry , during this week-end I will create a small bundle to add this functionality to earlier Symfony versions.

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Feb 9, 2018
…ateTimeType (vudaltsov)

This PR was merged into the master branch.

Discussion
----------

[Form] input=datetime_immutable for DateType, TimeType, DateTimeType

Related to symfony/symfony#25582.

Commits
-------

f6bf5f2 Added input=datetime_immutable for DateType, TimeType, DateTimeType.
@vudaltsov vudaltsov deleted the form-datetime-immutable branch February 11, 2018 10:17
@vudaltsov
Copy link
Contributor Author

Created a polyfill bundle for those who want to use input=datetime_immutable with Symfony (>=2.8 <4.1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants