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

Laravel 5.2.32 or greater breaks form #232

Closed
vincehollywood opened this issue Jun 1, 2016 · 23 comments
Closed

Laravel 5.2.32 or greater breaks form #232

vincehollywood opened this issue Jun 1, 2016 · 23 comments

Comments

@vincehollywood
Copy link

When updating to Laravel 5.2.32 or greater, I get the following error when trying to display a form with a button. Removing the button works.

Relationship method must return an object of type lluminate\Database\Eloquent\Relations\Relation

/* EditMediaForm.php */

public function buildForm()
{
     $this->add('filename', 'text')
         ->add('alt_attribute', 'text')
         ->add('save', 'submit', ['attr' => ['class' => 'btn btn-primary']]);
}
@kristijanhusak
Copy link
Owner

Hm, that error is really strange for something related to button. Are you attaching some model to the form?

@vincehollywood
Copy link
Author

Yeah. It's just a standard eloquent model. If I lower the Laravel framework to 5.2.31 it works fine.

On 1 Jun 2016, at 17:30, Kristijan Husak notifications@github.com wrote:

Hm, that error is really strange for something related to button. Are you attaching some model to the form?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@kristijanhusak
Copy link
Owner

That's really strange. Can you provide me the model content from $model->toArray() ?

@vincehollywood
Copy link
Author

$form = $formBuilder->create('App\Http\Forms\Backend\EditMedia',[
    'method' => 'PUT',
    'model' => $media->toArray(),
    'url' => route('admin.media.update', $media->id)
]);

Silly me. I didn't do the toArray(). However it's always worked just passing an object before 5.2.32.

@kristijanhusak
Copy link
Owner

It should work with object. I just need to see what your media model contains, it will help me debug the issue. Give me var_dump:

var_dump($media->toArray());

@vincehollywood
Copy link
Author

array(14) {
    ["id"] => int(2)
    ["filename"] => string(50) "13178822_10209364664818664_35521213679345507_n.jpg" 
    ["extension"] => string(3) "jpg" 
    ["mime_type"] => string(10) "image/jpeg"
    ["width"] => int(960)
    ["height"] => int(960)
    ["size"] => int(92926)
    ["source"] => string(23) "QcmV7uZD_2_Gw8NLKLm.jpg"
    ["alt_attribute"] => string(50) "13178822_10209364664818664_35521213679345507_n.jpg"
    ["album_id"] => int(2)
    ["cloned_id"] => NULL
    ["crop_data"] => NULL
    ["created_at"] => string(19) "2016-05-24 10:44:36"
    ["updated_at"] => string(19) "2016-05-24 10:44:36"
}

@vincehollywood
Copy link
Author

Works:

$form = $formBuilder->create('App\Http\Forms\Backend\EditMedia',[
    'method' => 'PUT',
    'model' => $media->toArray(),
    'url' => route('admin.media.update', $media->id)
]);

Fails:

$form = $formBuilder->create('App\Http\Forms\Backend\EditMedia',[
    'method' => 'PUT',
    'model' => $media,
    'url' => route('admin.media.update', $media->id)
]);

@kristijanhusak
Copy link
Owner

@vincehollywood
Copy link
Author

<?php

namespace App\Http\Forms\Backend;

use Kris\LaravelFormBuilder\Form;
use App\Models\Album;

class EditMedia extends Form
{   
    protected $album;

    public function __construct(Album $album)
    {
        $this->album = $album;
    }

    public function buildForm()
    {
        $albums = $this->album->get()->lists('title', 'id')->toArray();

        $this->add('filename', 'text')
            ->add('alt_attribute', 'text')
            ->add('album_id', 'select', [
                'choices' => $albums,
                'empty_value' => '-- Please choose --',
                'label' => 'Album'
            ])
            ->add('save', 'submit', ['attr' => ['class' => 'btn btn-primary']]);
    }

}

If I remove the $this->add('save', 'submit'), the form works perfectly fine. Albeit I can't submit the form.

@kristijanhusak
Copy link
Owner

I'll check it out. In the meantime, can you try removing album_id instead of save, and see if it works?

@vincehollywood
Copy link
Author

I've removed the album_id and I still get the error.

@kristijanhusak
Copy link
Owner

Ok thanks for your help. I'll check it out and let you know when i fix it. Until then, please use toArray().

@vincehollywood
Copy link
Author

No worries. One of my go to packages for any project. Keep up the good work!

rikless referenced this issue in illuminate/database Jun 3, 2016
* Refactor relations and scopes

* more refactoring

* styleCI
@rikless
Copy link

rikless commented Jun 3, 2016

May be related to illuminate/database@2d00f24 ?

@kristijanhusak
Copy link
Owner

@rikless how that could affect anything here?

@kristijanhusak
Copy link
Owner

@vincehollywood I found the issue. It's problem with the save field name.
It cannot be save, because i use field name to bind values to form from model, so when you pass eloquent model, and when form builder comes to save field, it tries to call $model->save, which throws error here https://github.com/illuminate/database/blob/master/Eloquent/Model.php#L2665 .
I believe you can just use something else (for example submit), and set label to Save. I'll add a check when fields are added, and to throw exception that is much more understandable. Closing this, since it not really a bug, just usage of reserved name.
Thanks for reporting.

@kristijanhusak
Copy link
Owner

I added a check in latest version (1.7.10), so other users get informed properly.

@JamesGuthrie
Copy link

JamesGuthrie commented Jun 7, 2016

The broken behaviour with 5.2.32 is/was a bug in the framework, which I reported here: laravel/framework#13632. It should be fixed in this commit which is in 5.2.36: laravel/framework@8fb89c6 (I have tried the combination of laravel/framework 5.2.36 and laravel-form-builder 1.7.0 and it worked). Can you revert the reserved field name check, as from 5.2.36 onwards the name is not 'reserved'.

@JamesGuthrie
Copy link

JamesGuthrie commented Jun 7, 2016

Actually it looks like I spoke too soon. Sorry. This issue: laravel/framework#13632 hasn't been fixed in 5.2.36.

@kristijanhusak
Copy link
Owner

kristijanhusak commented Jun 7, 2016

I'll leave it until we make sure it's fixed in the latest release. I'll reopen it for now.

@kristijanhusak kristijanhusak reopened this Jun 7, 2016
@JamesGuthrie
Copy link

The underlying issue has not and most likely will not be fixed, so I guess you can close this issue (the closed pull request for reference: laravel/framework#13750)

@csogilvie
Copy link

Hi, @kristijanhusak - if this change is contined to be supported, can the documentation at http://kristijanhusak.github.io/laravel-form-builder/field/buttons.html please be updated as it shows a save button by default... and has caused me at least 15 minutes of frustration so far trying to work out why I was getting an error when following the documentation :)

@kristijanhusak
Copy link
Owner

@csogilvie i fixed it, 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

5 participants