Skip to content

Feature: Add In, InStrict, and DateTime validation rules #10

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

Merged
merged 3 commits into from
Mar 16, 2023

Conversation

JasonTheAdams
Copy link
Member

@JasonTheAdams JasonTheAdams commented Mar 16, 2023

Resolves #1

This adds the In and DateTime validation rules.

In Rule

The In rule limits a value to a given set of values:

'agree' => ['in:yes,no'],
'rating' => [new In(1, 2, 3, 4, 5)]

The only thing I'm going back and forth on is whether value checking should be strict or not. If it's not strict, then in:1,2,3 wouldn't match integer values of 1, 2, 3. That seems counterintuitive, but is only an issue when converting from string values. Still, that's encouraged behavior. Looking at the Laravel In rule, it's not strict. If not strict, then perhaps the In constructor should not be variadic and have a second, $strict parameter: new In([1, 2, 3], true) — which is false by default. Open to thoughts!

DateTime Rule

The DateTime rule validates that a given value is a valid date and/or time, and then sanitizes it as a DateTimeImmutable:

'flexibleDateTimeFormat' => 'dateTime',
'specificDateTimeFormat' => 'dateTime:Y-m-d H:i:s'

Note that relative date formats never pass. So values like now or +1 day will not pass validation.

If the value passed is a DateTimeInterface instance, then it passes and is directly passed back. Otherwise, the value is parsed to a DateTimeImmutable (using the optional format if provided), and the new instance is returned.

It may be too opinionated to return a DateTimeImmutable over a DateTime. I'm open to feedback here as well.

@borkweb
Copy link
Member

borkweb commented Mar 16, 2023

In Rule

For a validation library, it seems smart to provide the ability to be strict with the types. I can see the value in the flexibility of non-strict checking, though. For that reason, I'd lean toward the non variadic approach or add a StrictIn() class that extends and overrides In().

For the non-strict approach, I imagine we would want to cast as (string) the value in the in_array() call in __invoke() - similar to what Laravel is doing.

Additionally, it'd be good to get a couple of tests centered around the use case you laid out in your PR description:

  • in:1,2,3
  • new In(1, 2, 3)
  • new In("1", "2", "3")
  • new In("1", 2, "3")

DateTime Rule

I'm a fan of the use of DateTimeImmutable over DateTime.

@JasonTheAdams
Copy link
Member Author

Thanks @borkweb!

For that reason, I'd lean toward the non variadic approach or add a StrictIn() class that extends and overrides In().

Yeah, I was leaning towards a StrictIn rule, that makes it clear and it can also be strict or non-strict in the string version. I'll do that. 👍

For the non-strict approach, I imagine we would want to cast as (string) the value in the in_array() call in __invoke() - similar to what Laravel is doing.

Yeah, I think that should work. Honestly, I'm not entirely sure what that's accomplishing since "2" == 2 — whichever side the values are on.

@JasonTheAdams JasonTheAdams requested a review from borkweb March 16, 2023 18:02
@JasonTheAdams
Copy link
Member Author

JasonTheAdams commented Mar 16, 2023

Hey @borkweb!

I added the InStrict rule, changed In to be loose, and I'm happy with how it works now! Please give this another review!

Copy link
Member

@borkweb borkweb left a comment

Choose a reason for hiding this comment

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

Glorious 😍

@JasonTheAdams JasonTheAdams changed the title Feature: Add In and DateTime validation rules Feature: Add In, InStrict, and DateTime validation rules Mar 16, 2023
@JasonTheAdams JasonTheAdams merged commit 0da4b81 into develop Mar 16, 2023
@JasonTheAdams JasonTheAdams deleted the feature/in-and-datetime-rules branch March 16, 2023 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add additional validation rules
2 participants