Skip to content

[Validator] Added DateTimeRangeValidator #9164

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 0 commits into from
Closed

[Validator] Added DateTimeRangeValidator #9164

wants to merge 0 commits into from

Conversation

FineWolf
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR (symfony/symfony-docs#3019)

This adds a new validator to verify if a DateTime is between an optional min or max. Essentially, it is the Range validator for DateTime.

Travis failed due to c741c5838d2115d2ac5c50e77c6aef049199c0f3

@FineWolf FineWolf mentioned this pull request Sep 29, 2013
Closed
@hacfi
Copy link
Contributor

hacfi commented Sep 29, 2013

FYI: @Exelenz has been working on the same feature: #7766

@FineWolf
Copy link
Contributor Author

This proposal has a few advantages over #7766...

  1. This proposal supports min and max to be specified as both DateTime objects and strings. This allows for greater precision when creating a constraint from within PHP, while keeping compatibility with other types of constraint configuration providers.

  2. This proposal allows any value that passes the Date and DateTime validators to be compared. [Validator] DateRange constraint added #7766 only validates if the value being validated is an instance of DateTime. This allows for more consistency between already existing constraints and this new one (why would Date consider a string representing a proper date as valid but not DateTimeRange)?

  3. Contrary to [Validator] DateRange constraint added #7766, the timezone option ensures that the timezone of both the value and the min/max options is never undefined (this was an issue raised by @bschussek ). The timezone is by default set to UTC, but can be changed if needed. Otherwise, the min/max options can be specified in full ISO 8601 to force a timezone other than the timezone option.

  4. This proposal supports relative formats (as accepted by the DateTime constructor). For example, to verify if the age of a person is at least 18 years old, you may specify the following constraint (using Annotations for example):

    /**
 * @Assert/DateTimeRange(
 *     max="midnight 18 years ago",
 *     maxMessage="You must be 18 years or older."
 * )
 */

  5. This proposal is unit tested and documented already.

@sstok
Copy link
Contributor

sstok commented Sep 29, 2013

CONTRIBUTORS.md is automatically updated 👍

stloyd
stloyd reviewed Sep 29, 2013
View reviewed changes
CONTRIBUTORS.md
@@ -881,3 +881,4 @@ Symfony2 is the result of the work of many people who made the code better
- Erik Saunier (snickers)
- Tony Piper (tonypiper)
- Vladislav Vlastovskiy (vlastv)
- Andrew Moore (FineWolf)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be reverted. It's generated automatically by Fabien from time to time.

@FineWolf
Copy link
Contributor Author

@stloyd @sstok : Changes made as per commented (and more instances of the same problems).

stof
stof reviewed Sep 30, 2013
View reviewed changes
src/Symfony/Component/Validator/Constraints/DateTimeRange.php
throw new InvalidOptionsException(sprintf(
'Option "timezone" must be a valid TimeZone for constraint %s',
__CLASS__
), array('timezone'));
Copy link
Member

Choose a reason for hiding this comment

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

You should link the catched exception as previous exception

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, however this required a change to the signature of InvalidOptionsException in order to support linked exceptions (the \Exception class has no setter for this, and it has to be set while constructing the exception).

This change does not break BC.

@bamarni
Copy link
Contributor

bamarni commented Sep 30, 2013

This proposal allows any value that passes the Date and DateTime validators to be compared.

Then why creating new validators? see #8300, it still misses a few tweaks (timezone + min/max messages) but it's a lighter approach.

@FineWolf
Copy link
Contributor Author

@bamarni : Because of the single responsibility principle.

  • Date is used to validate if a passed string or DateTime object is a valid date.
  • DateTime is used to validate if a passed string or DateTime object is a valid date time.
  • DateTimeRange will be used to validate if a passed string or DateTime object passed within the specified Range.

If you look at the Range validator, you will also notice that it isn't used for number validation (ie.: you cannot just use Range without specifying at least a min or a max).

@FineWolf
Copy link
Contributor Author

Travis failed due to c741c5838d2115d2ac5c50e77c6aef049199c0f3... Test failures are unrelated to this pull request.

@fabpot : Can you take a look at your changes?

@bamarni
Copy link
Contributor

bamarni commented Oct 3, 2013

@FineWolf : imo your comparison with Range is inaccurate, I'm not suggesting to make a DateRange constraint validating a date, but to put those options into Date. I think that the reason why Range exists on its own is because there is no Number constraint, and it wouldn't make any sense to have a min and max options in Type.

About single responsibility principle, if those constraints have totally different purposes, why half of your new validator's class is a duplication from date and datetime validators (for datetime object creation)?

@FineWolf FineWolf closed this Oct 7, 2013
@bamarni bamarni mentioned this pull request Nov 28, 2013
Closed
@webmozart webmozart mentioned this pull request Aug 15, 2014
Merged
nicolas-grekas added a commit that referenced this pull request Aug 24, 2014
@nicolas-grekas
feature #11673 [Validator] Added date support to comparison constrain…
1d7684a 
…ts and Range (webmozart)

This PR was merged into the 2.6-dev branch.

Discussion
----------

[Validator] Added date support to comparison constraints and Range

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #3640, #7766, #9164, #9390, #8300
| License       | MIT
| Doc PR        | symfony/symfony-docs#4143

This commit adds frequently requested functionality to compare dates. Since the `DateTime` constructor is very flexible, you can do many fancy things now such as:

```php
/**
 * Only accept requests that start in at least an hour.
 * @Assert\GreaterThanOrEqual("+1 hours")
 */
private $date;

/**
 * Same as before.
 * @Assert\Range(min = "+1 hours")
 */
private $date;

/**
 * Only accept dates in the current year.
 * @Assert\Range(min = "first day of January", max = "first day of January next year")
 */
private $date;

/**
 * Timezones are supported.
 * @Assert\Range(min = "first day of January UTC", max = "first day of January next year UTC")
 */
private $date;
```

Commits
-------

60a5863 [Validator] Added date support to comparison constraints and Range
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Feature Validator
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@FineWolf @hacfi @sstok @bamarni @stloyd @stof