Laravel-Backpack / CRUD

Build custom admin panels. Fast!
https://backpackforlaravel.com
MIT License
3.08k stars 886 forks source link

[Bug] Backpack v5 breaks morphTo select field #4323

Closed stevebeyatte closed 1 year ago

stevebeyatte commented 2 years ago

Bug report

What I did

I have a n-1 polymorphic relationship between "Intros" and "Companies" or "People". Meaning an intro can link a user to either a company or a person. I have an 'introable_id' column in the intros table that points to the related model. The relationship is defined on App\Models\Intro as:

public function introable() { return $this->morphTo(); }

I have been using the select2 (1-n) relationship field in Laravel Backpack v4 without issues by just hardcoding the model that I want to default to from Backpack:

$this->crud->addField([  // Select2
            'label'     => "Intro To",
            'type'      => 'select2',
            'name'      => 'introable_id', 
            'entity'    => 'introable',
            'model'     => "App\Models\Company",
            'attribute' => 'name', 
        ]);

What I expected to happen

I thought I could update to Laravel Backpack v5 and this would still work.

What happened

After updating, creating a new Intro or updating an existing one throws an error. On create, I get: Field 'introable_id' doesn't have a default value even though I've double-checked that the request does indeed have introable_id set properly.

On update, I get: Call to undefined method App\Models\Intro::introable_id() The relationship is called 'introable' but the db column is called introable_id. This is thrown because line 99 in the Create trait is: $relation = $item->{$relationMethod}();

What I've already tried to fix it

I tried to port over the v4 version of Select2 and that didn't work. I verified that the request has the proper data and searched around a lotl

Is it a bug in the latest version of Backpack?

Yes

Backpack, Laravel, PHP, DB version

When I run php artisan backpack:version the output is:

PHP VERSION:

PHP 8.1.1 (cli) (built: Dec 17 2021 22:38:05) (NTS) Copyright (c) The PHP Group Zend Engine v4.1.1, Copyright (c) Zend Technologies with Zend OPcache v8.1.1, Copyright (c), by Zend Technologies

LARAVEL VERSION:

v8.83.8@cf430301ad17656b3d918995bcdd0454c3c119b9

BACKPACK VERSION:

5.0.14@1e355c4c046a34423a1a3e3150120245a4bfd8e9

welcome[bot] commented 2 years ago

Hello there! Thanks for opening your first issue on this repo!

Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.

Backpack communication channels:

Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.

Thank you!

-- Justin Case The Backpack Robot

pxpm commented 2 years ago

Hello @stevebeyatte

I am working on a core solution for the morph fields, I should have an mvp PR ready by tomorrow. In the meanwhile use entity => false on your introable_id field.

Let me know if that solves it for you.

Cheers

stevebeyatte commented 2 years ago

@pxpm thanks so much for your help. When I do that I get "Undefined array key "relation_type" on vendor/backpack/crud/src/app/Library/CrudPanel/Traits/Input.php:123

tabacitu commented 2 years ago

That's the thing - if you specify entity => false then you'll have to manually give the field everything it needs. But after you do that, it should work as it did before.

relation_type should be MorphTo in your case, right? (string)

stevebeyatte commented 2 years ago

@tabacitu @pxpm When I specify relation_type I also get the same query builder error, screenshot attached.

$this->crud->addField([  // Select2
            'label'     => "Intro To",
            'type'      => 'select2',
            'name'      => 'introable_id',
            'relation_type' => 'MorphTo',
            'entity'    => false, 
            'model'     => "App\Models\Company", 
            'attribute' => 'name', 
        ]);

It calls the function off of the name attribute above, ex $model->introable_id(). I changed the name to introable (which is how the polymorphic relationship is defined) but then the introable_id field doesn't get passed so the database chokes without that value.

image

stevebeyatte commented 2 years ago

Thoughts on a quick fix for this @pxpm ?

pxpm commented 2 years ago

@stevebeyatte It needs some core changes since introable_id is detected as a relationship (MorphTo) when it shouldn't, the relation is introable_id + introable_type.

As a workaround you can use other field name, like introable_input.

Then add a mutator and getter for your fake field that adds the real attribute in the model:

public function setIntroableInputAttribute($value) {
    $this->attributes['introable_id'] = $value;
}

public function getIntroableInputAttribute() {
    return $this['attributes']['introable_id'];
}

If you are using $fillable you should also add the introable_input into the fillable array.

If you run into problems using select2 field, I'd recommend you to use the select2_from_array since you'd be populating the select with javascript depending on the type, just use the select2_from_array with empty [] options and then populate it with JS.

I am sure I've already set morphTo relations working like this, you should be fine 👍

When the PR gets merged you just need to CRUD::field('introable')->models(['\App\Model\1',\App\Model\2']); .

Let me know, Cheers

adrienne commented 2 years ago

@pxpm @tabacitu any progress on MorphTo (not MorphToMany) relationships being handled either this way or by the relationship field type? I really need plain MorphTo, one way or another.

tabacitu commented 2 years ago

@adrienne we're working on it right now. ETA: 2 weeks

stevebeyatte commented 2 years ago

friendly ping on this one :-)

tabacitu commented 2 years ago

@stevebeyatte , @adrienne I've just finished reviewing the new morphTo field inside repeatable. It is a BEAUTY: https://github.com/Laravel-Backpack/docs/pull/368

So simple, yet so powerful.


It's going through one more round of testing, by @jorgetwgroup . We expect to have it merged in 1-2 weeks, max.

Thank you for your patience 🙏

tabacitu commented 2 years ago

@pxpm , I believe we can close this, since we'll soon be merging all PRs related to this?

tabacitu commented 1 year ago

professor-farnsworth-futurama-gif

I've just merged https://github.com/Laravel-Backpack/CRUD/pull/4579 (and the corresponding PRO PR) which add morphTo functionality to the relationship field 😱 It's been a multi-month effort by @pxpm , and it hasn't been easy at all, so all credits go to him 👏👏👏

It's currently on main, where we'll test it a little more until Monday... and on Monday we tag 3.4.0 and launch it 🎉

I hope you'll be as happy to have this as we are. Thank you for your patience 🙏

Cheers!