Skip to content
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

Filling fields which depend on another field depends on the order of field definition in the schema #65

Closed
X-Coder264 opened this issue Apr 7, 2021 · 3 comments
Milestone

Comments

@X-Coder264
Copy link
Contributor

X-Coder264 commented Apr 7, 2021

When the filling logic of a field depends on another field we can use the fillUsing method of an attribute to customize the filling logic. In doing so the order of the field definitions matters as the hydrator goes in order and hydrates each field.

Laravel Nova gets the entire request when it hydrates a field:

Text::make('Name', 'name')
    ->fillUsing(function ($request, $model, $attribute, $requestAttribute) {
        $model->{$attribute} = Str::title($request->input($attribute));
    }),

We could pass the entire validated request data in the fillUsing Closure too, so that we don't have to depend on the field definition order.

@X-Coder264 X-Coder264 changed the title Filling fields which depend on another field depend on the order of field definition in the schema Filling fields which depend on another field depends on the order of field definition in the schema Apr 7, 2021
@lindyhopchris lindyhopchris added this to the v1.0.0-beta.2 milestone Apr 7, 2021
@lindyhopchris
Copy link
Contributor

Just to note what I said on Slack... we don't want to pass the whole request, because we will need to decouple from requests in the near future to support #39 Atomic Operations.

So the solution is probably invoking the fillUsing hydrator like this:

if ($this->hydrator) {
    ($this->hydrator)($model, $this->column(), $value, $validatedData);
    return;
}

But need to check before implementing.

@lindyhopchris
Copy link
Contributor

@X-Coder264 this is now on develop, which if you allow dev dependencies you can install by:

composer require laravel-json-api/laravel:^1.0.0-beta.2

I've gone with passing the validated data as the fourth argument, as per my comment above. Would be good to get confirmation from you that it works well for your use case!

@X-Coder264
Copy link
Contributor Author

@lindyhopchris Sorry for the late response (I've had a night deploy today 😄 ). I've tried it out and it works as expected, thanks 🎉

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

No branches or pull requests

2 participants