Skip to content

[5.6] Allow to pass collection to validation rules#23875

Merged
taylorotwell merged 1 commit intolaravel:5.6from
jerguslejko:allow-collections-in-rules
Apr 17, 2018
Merged

[5.6] Allow to pass collection to validation rules#23875
taylorotwell merged 1 commit intolaravel:5.6from
jerguslejko:allow-collections-in-rules

Conversation

@jerguslejko
Copy link
Copy Markdown
Contributor

This PR makes necessary changes to allow for passing Illuminate\Support\Collection objects to Rule::in() and Rule::notIn() methods.

Thanks

@jerguslejko jerguslejko changed the title Allow to pass collection to validation rules [5.6] Allow to pass collection to validation rules Apr 13, 2018
*/
public static function in($values)
{
if ($values instanceof Collection) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe this should be generalized to Illuminate\Contracts\Support\Arrayable that Collection implements?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We'd need to rewrite In and NotIn rules to accept array and collections then.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Since Arrayable contains the toArray method all you would need to change was the condition I believe.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh I see you point now. Sorry guys.

I'd argue that Arrayable is implemented by following classes: Request, LengthAwarePaginator, MessageBag, Fluent etc.

Still think that Collection & array are the way to go.

@taylorotwell taylorotwell merged commit e3d958b into laravel:5.6 Apr 17, 2018
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.

4 participants