Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

Strict mode for Date validator #275

Merged
merged 4 commits into from
Dec 28, 2019
Merged

Strict mode for Date validator #275

merged 4 commits into from
Dec 28, 2019

Conversation

michalbundyra
Copy link
Member

Input must be always in the defined format and must be the same as output
of format method of DateTime.

Strict mode is set to false by default to keep BC.

Currently to the validator we can provide integer, double, array, string or DateTimeInterface value. In my opinion as validator has set format we should check if the input value has exactly the same format as set.
In strict mode we are comparing if:

$input === DateTime::createFromFormat($format, $input)->format($format);

Resolve #33
Fix zendframework/zendframework#6407

  • Are you creating a new feature?
    • Why is the new feature needed? What purpose does it serve?
    • How will users use the new feature?
    • Base your feature on the develop branch, and submit against that branch.
    • Add only one feature per pull request; split multiple features over multiple pull requests
    • Add tests for the new feature.
    • Add documentation for the new feature.
    • Add a CHANGELOG.md entry for the new feature.

I would see strict mode by default enabled in v3, or ONLY strict mode in v3.
Thoughts?

/cc @svycka @gianarb

Copy link
Contributor

@svycka svycka left a comment

Choose a reason for hiding this comment

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

maybe in v3, we could leave the only strict mode?

src/Date.php Outdated
* @param bool $strict
* @return $this
*/
public function setStrict($strict)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe drop php5.6 and add boolean type-hint?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not in v2.

['6', 'd', true, false],
['06', 'd', true, true],
[123, null, true, false],
[1340677235, null, true, false],
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe also add Unix timestamp with a format here?

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem here is that input argument must be string even for U format. Output of DateTime::format is always a string.

Copy link
Member

@froschdesign froschdesign left a comment

Choose a reason for hiding this comment

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

The documentation for the new strict mode is needed. The following points should be included:

  • Why is the strict mode required?
  • Which mode should be preferred?
  • And like always: code examples

michalbundyra and others added 4 commits December 27, 2019 21:44
Input must be always in the defined format and must be the same as output
of format method of DateTime.

Strict mode is set to `false` by default to keep BC.

Resolve #33
Fix zendframework/zendframework#6407
Updates new methods of Date validator and its tests to use typehints.

Updates license docblock year range.
Copy link
Member

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

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

Since #264, the develop branch now targets PHP 7.1. As such, I've pushed changes to add typehints to the newly introduced methods, which will prevent BC breaks in the future, and simplifies their implementation.

I've also added documentation on the new strict mode.

weierophinney added a commit that referenced this pull request Dec 28, 2019
@weierophinney weierophinney merged commit 8bdb9db into zendframework:develop Dec 28, 2019
@michalbundyra michalbundyra deleted the feature/date-validator-strict-mode branch December 28, 2019 04:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants