-
Notifications
You must be signed in to change notification settings - Fork 11k
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
[8.x] Validated subsets #38366
[8.x] Validated subsets #38366
Conversation
Decided to change approach and add a new You can now call the |
Loving it! It effectively resolves something I had to code just a few hours ago ⭐️ |
@taylorotwell Should the interface contain the only() and except() methods? 🤔 |
*/ | ||
public function safe() | ||
{ | ||
return new ValidatedInput($this->validated()); |
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.
i have a question ... may i know why not bind the interface to the concrete ValidatedInput instance inside ValidationServiceProvider ... and here call app(ValidatedData::class, ['input' => $thsi->validated()])
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.
That's nice! And also changing the return type to the interface. I mean, we don't need the interface if the implementation is hardcoded to a specific class.
$results = []; | ||
|
||
$input = $this->input; | ||
|
||
$placeholder = new stdClass; | ||
|
||
foreach (is_array($keys) ? $keys : func_get_args() as $key) { | ||
$value = data_get($input, $key, $placeholder); | ||
|
||
if ($value !== $placeholder) { | ||
Arr::set($results, $key, $value); | ||
} | ||
} | ||
|
||
return $results; |
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.
Any reason we aren't just hitting the Arr::only()
, Arr::*
method on all of these?
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.
I noticed that Request's InteractsWithInput
had it's own implementation that I presume has some slightly different behavior than a plain Arr::only
. Therefore, I just kept the implementation matching. I haven't dug into what the subtle differences between the two would be.
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.
one difference that I can think of is passing *
would return different result for both @timacdonald @taylorotwell
public function collect() | ||
{ | ||
return new Collection($this->input); | ||
} |
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.
@ryangjchandler will appreciate this one
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.
This is beautiful.
This is rad. Only thing I would add to this is that it could be nice and also consistent to allow property style access on the $request->safe()->name;
$request->safe('name'); This is already possible with array access Of course that could come in a follow up or something we implement in our own applications. Just a thought! Thanks again! |
Added |
@franzliedke the reason I didn't add those to the interface was the main use case I could think of for the interface was Eloquent mass-assignment operations, where you would simply be iterating through the data and hydrating a model without checking the fillable or guarded arrays. In that use case and similar use cases only iteration is needed. |
* @return void | ||
*/ | ||
#[\ReturnTypeWillChange] | ||
public function offsetSet($key, $value) |
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.
Wouldn't an immutable object make sense? A stict guarantee that ValidatedData is the unmodified result of a validator would sometimes be usefull i guess. What i mean is, changing a validated input may make it not the valid anymore.
@taylorotwell Any plan adding this feature in UPDATE: NVM, i've just checked the changed files. Thanks taylor ❤️ |
*/ | ||
public function safe() | ||
{ | ||
return new ValidatedInput($this->validated()); |
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 instead:
return $this->validator->safe();
And you can remove the use statement after.
Co-authored-by: Mior Muhammad Zaki <[email protected]>
@taylorotwell @timacdonald Thank you for your efforts! |
use Illuminate\Contracts\Support\ValidatedData; | ||
use stdClass; | ||
|
||
class ValidatedInput implements ValidatedData |
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.
Any specific reason this isn't an actual collection?
(The only difference is the only
method working on nested data, which we could easily add to the collection's method, or just override it here)
Convenient methods for getting subsets of validated data.