8000 [Validator] Replace Date, DateTime and Time by Timestamp · Issue #11925 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Validator] Replace Date, DateTime and Time by Timestamp #11925

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

Closed
webmozart opened this issue Sep 15, 2014 · 11 comments
Closed

[Validator] Replace Date, DateTime and Time by Timestamp #11925

webmozart opened this issue Sep 15, 2014 · 11 comments
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed) Validator

Comments

@webmozart
Copy link
Contributor

At the moment, we have three constraints for validating date/time values:

  • Date accepts \DateTime instances and 'Y-m-d' strings
  • DateTime accepts \DateTime instances and 'Y-m-d H:i:s' strings
  • Time accepts \DateTime instances and 'H:i:s' strings

I think that's a bit confusing. Usually, a value should not be a \DateTime or a string, but either of them; if it is a string, it should usually match a specific pattern.

Therefore I propose to deprecate all three constraints. The \DateTime case can be solved with the Type constraint already:

/**
 * @Assert\Type('\DateTime')
 */
private $createdAt;

The second case would be covered by a new Timestamp constraint which accepts a format as argument as supported by PHP:

/**
 * @Assert\Timestamp('Y-m-d')
 */
private $createdAt;

/**
 * @Assert\Timestamp('c')
 */ 
private $iso8601Date;

This would fix #11919 and replace #11821.

@stof
Copy link
Member
stof commented Sep 15, 2014

👍 for this. The current string validation is quite useless as it does nto allow changing the format of the string.

what would be the behavior if you don't configure a format ? Accepting any format supported by the DateTime constructor, throwing an exception because the format is required, or using a default format ?

@webmozart
Copy link
Contributor Author

I'd make format required. What do you think?

@stof
Copy link
Member
stof commented Sep 15, 2014

yeah, this seems good to me.

@fabpot
Copy link
Member
fabpot commented Sep 15, 2014

Sounds good to me as well.

@lmammino
Copy link

Great solution! 👍 x 1000 for it 😉

@Tobion
Copy link
Contributor
Tobion commented Sep 18, 2014

Accepting any format supported by the DateTime constructor

If this is not be the default behavior, how else would we allow this use-case to be validated then?

@stof stof added Validator RFC RFC = Request For Comments (proposals about features that you want to be discussed) labels Sep 20, 2014
@hhamon
Copy link
Contributor
hhamon commented Nov 8, 2014

+1

@gimler
Copy link
Contributor
gimler commented Mar 9, 2015

actually i run into this problem @webmozart do you have time to implement it? if not i can do this for symfony 3.0.

@Jean85
Copy link
Contributor
Jean85 commented Mar 9, 2017

I agree with @Tobion, this change will make not possible to accept multiple formats for the same input..

@lepiaf
Copy link
Contributor
lepiaf commented Mar 15, 2017

As @javiereguiluz said in #21905 (comment), what do you think about reworking DateTimeValidator to include this change and deprecate TimeValidator and DateValidator?

@nicolas-grekas
Copy link
Member

Closing as this is old, and @Tobion raised a point that is not answered since years.

fabpot added a commit that referenced this issue Sep 8, 2018
…Date|Time|DateTime constraints (ro0NL)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Validator] Deprecate validating DateTimeInterface in Date|Time|DateTime constraints

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #11925
| License       | MIT
| Doc PR        | symfony/symfony-docs#7583 (old PR but not really needed now)

Easy version of #21905. I think individual naming has value. Also the goal is to move forward to use `Type` really, not to bother with constraint renames.

Commits
-------

5454e6f [Validator] Deprecate validating DateTimeInterface in Date|Time|DateTime constraints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed) Validator
Projects
None yet
Development

No branches or pull requests

10 participants
0