kristijanhusak / laravel-form-builder

Laravel Form builder for version 5+!
https://packagist.org/packages/kris/laravel-form-builder
MIT License
1.7k stars 297 forks source link

Embedded child form models being converted to arrays #184

Open hackel opened 8 years ago

hackel commented 8 years ago

I am trying to add a child form that uses a related model.

$this
            ->add('name', 'text')
            ->add('directories', 'form', [
                'class'       => 'DirectoriesForm',
                'formOptions' => [
                    'model' => $this->model ? $this->model->directories : null,
                ],
            ])

In \Kris\LaravelFormBuilder\Form::setupNamedModel it is converting the child model to an array, so I can't call any methods on it. Why is this? It prevents me from calling any model methods in the child form. As far as I know, I'm not even using a named form, unless this happens automatically for child forms. If I get rid of the dotName stuff and just set

            $this->model = [
                $this->getName() => $this->getModel()
            ];

it seems to work fine. Am I doing something wrong here? Thanks.

hackel commented 8 years ago

Also, if I pass a form instance instead of passing a string like so: 'class' => $this->formBuilder->create('DirectoriesForm', ['model' => $this->model ? $this->model->directories : null]), something very strange happens. It first passes the model instance to the child form, but then it calls the child's buildForm method a second time, but with $this->model equal to the arrayed model.

I actually have a third level of child forms I'm trying to add, so if I only have an array to work with, I can't retrieve the child model instances to pass to the grandchild form, unless I were to explicitly re-retrieve the model from the DB.

kristijanhusak commented 8 years ago

That is mostly because of this issue https://github.com/kristijanhusak/laravel-form-builder/issues/157 . Basically problem occurs when you try to add named form as child form, like you did in 2nd comment, or when you manually add a model for child form. In that situation, if we do not pass namespaced model (do an array_set), Laravel's Form class will not know how to bind the values to the form. If for example field name in child form has name like form[child_form][second_child_form][field_name], Laravel's Form class expects this data structure:

$model = [
    'child_form' => [
        'second_child_form' => [
            'field_name' => 'field value'
        ]
    ]
];

And if you pass a model to your child form, it may not have that namespace in it. It will probably have something like ['field_name' => 'field_value']. Than I have to manually namespace data so the value binding is done properly later.

Ideally, all data for all child forms would be pulled with main model, so you do not have to do that manually. Your directories child form shouldn't even need manual model binding, if the main form model has that property directories in it.

Is there any reason why you bind it manually?

For your 2nd comment, buildForm is run twice because at first moment when you do instantiation with create(), it calls buildForm, and when is added as child form, I must rerun buildForm again to set up new namespace for the fields, because child form automatically sets name property for that form in the background.

hackel commented 8 years ago

Actually I settled on removing the manual model binding, and it's able to fill all of the fields just fine.

The problem remains that I can't access the actual model (either child or parent) from the (grand)child form, getModel only returns the array. The array structure doesn't make any sense to me, though. In order to access the parent model within the grandchild form, I'm currently doing this: $parentModel = Repository::find(array_get($this->getModel(), 'parent.child.grandchild.parent.child.id')); I suspect it looks like that because, before the dotName conversion was added, it looked like:

['parent[child][grandchild]' => [
    'parent[child]' => [
        'id' => ...
    ]
]]

I suppose I could pass the parent model along in the data field to eliminate the redundant DB hit, but it seems like I should just be able to do something like $this->getParent()->getModel().

kristijanhusak commented 8 years ago

I know it doesn't make any sense, but that's the only way I can make form model value binding work for child forms. Do you have any idea how I could achieve "namespacing" data? So for example, if you have this data (i'll write model as array because it's simpler, but imagine it's an Eloquent model):

$model = [
    'first_name' => 'John'
    'last_name' => 'Doe'
];

to become something like this if child form has name user:

$model = [
    'user' => [
        'first_name' => 'John'
        'last_name' => 'Doe'
    ]
];

so everything needs to be an object. and something like this should be possible to make it work:

$model->user->first_name;

I was thinking about using just blank StdClass, but it's not ideal. Do you maybe have some solution?

hackel commented 8 years ago

Hmm, I'm using laravel-mongodb, which actually embeds the child models in the parent model by storing them in the $relations property. Of course this would really over-complicate things for something like this.

I just realised I could just hydrate an empty model with the array data to get my model back without an additional DB query, so I'm going to give that a go.

kristijanhusak commented 8 years ago

Yes, that's a good solution, only one for this one unfortunately.