-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[5.4] Add Request::pick() #17842
[5.4] Add Request::pick() #17842
Conversation
What's the difference between |
Output of request()->only('name', 'category', 'level', 'age')' will be:
|
Does it really make sense to have 3 different methods that each work slightly differently? |
|
||
return collect($this->all())->filter(function ($value, $key) use ($keys) { | ||
return in_array($key, $keys); | ||
})->toArray(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this?
return array_intersect_key($this->all(), array_flip($keys));
@JosephSilber I personally think |
If |
I personally never cared for the current I do however see many people on SO constantly being frustrated that it works the way it currently does. It's counter-intuitive and in stark contrast with If I'm correct that needing the old functionality is an edge case, we don't have to provide a separate method for that. If someone really needs it, they can get similar functionality with this monstrosity: $keys = ['some', 'keys'];
$data = array_merge(array_fill_keys($keys, null), request()->only($keys)); And since the request is now macro-able, if people need this throughout their project they can create their own method that encapsulates it. To summarize, I believe:
|
I personally think the |
So, now we have a new debate over what should be pick, what should be only, and what should be intersect? |
@taylorotwell I think the three of us think that |
I like the A lot of our code, we safely assume the array index exists after the
Now we can safely use these index without worrying about
|
@yadakhov you can always add that functionality in a macro, and replace your existing |
I agree with @yadakhov, why change only() method, when everywhere in the examples we see only() as an example, and now it gets changed? we often need empty values also, since user can remove data, so if the field value is '' it does not mean anything, and database should be updated. |
@huglester the new |
Then I am happy user :) |
This will be very helpful in effectively implementing patch requests. |
So what is our best course here? Should we make request->only behave like request->pick does here for Laravel 5.5? We would maybe need to add a method that works like current request->only() (perhaps that could be request->pick()?) for backwards compatibility. I'm going to close this on 5.4 as I don't think we'll do anything on the 5.4 release but want to talk about what we should do for 5.5. |
The method works like
intersect()
, however the problem with intersect is that it excludes null, empty, and zero values.So for example a payload like:
Output of
request()->intersect('name', 'category', 'level', 'age')'
will be an empty array.Output of
request()->only('name', 'category', 'level', 'age')'
will be:Output of
request()->pick('name', 'category', 'level', 'age')'
will be: