Crud: [Bug] $entity_model::isColumnNullable() broken for models that extend abstract class that uses CrudTrait

Created on 23 Jul 2020  路  3Comments  路  Source: Laravel-Backpack/CRUD

Bug report

What I did

Upgraded to v4.1 following instructions in documentation.

What I expected to happen

I expected my CrudControllers for my model classes that extend a common abstract model class to continue working as they did for v3.6 and v4.0.

What happened

Use of the select2 field (and similar/related fields) causes a Cannot instantiate abstract class exception. When new self is called in HasRelationshipFields::getConnectionWithExtraTypeMappings(), "self" references the abstract class rather than actual CRUD model.

What I've already tried to fix it

I've bypassed my abstract class to extend Eloquent and use CrudTrait directly. Doing this for all of my crud models would allow the app to function, but it would also add a lot of boilerplate to each model.

The exception traces to the select2 field's use of isColumnNullable(), which was a CrudTrait method in previous versions. Any other field that uses isColumnNullable() is definitely affected by this. (see: https://github.com/Laravel-Backpack/CRUD/blob/4.0.61/src/app/Models/Traits/CrudTrait.php#L54)

That path ultimately leads to HasRelationshipFields::getConnectionWithExtraTypeMappings() which appears to needlessly re-instantiate after having been called on an instance. Basically, shouldn't this line -- https://github.com/Laravel-Backpack/CRUD/blob/4.1.15/src/app/Models/Traits/HasRelationshipFields.php#L25 -- just use $this->getConnectionName()?

Backpack, Laravel, PHP, DB version

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

PHP VERSION:

PHP 7.3.5-1+ubuntu18.04.1+deb.sury.org+1 (cli) (built: May 3 2019 10:00:24) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.3.5, Copyright (c) 1998-2018 Zend Technologies
with Zend OPcache v7.3.5-1+ubuntu18.04.1+deb.sury.org+1, Copyright (c) 1999-2018, by Zend Technologies

LARAVEL VERSION:

v7.21.0@3ccdb116524de408fdc00715b6f06a1031ddace9

BACKPACK VERSION:

4.1.15@6c751de946a9c8511dd32eb7bfa3ca6a568849f5

Minor Bug Possible Bug

All 3 comments

If this small change is more problematic than it seems, a smaller work-around that minimizes my boilerplate issue would be to just move the use CrudTrait; out of the abstract and into each model.

I'll let @pxpm investigate, he knows this part of the code better than me, but I just wanted to say @trip-somers - excellent bug report, thank you!

Hello @trip-somers

Sorry for taking so much time to get into this one.

I'v checked and we only call getConnectionWithExtraTypeMappings with initialized instances so we are good to remove the "re-initialization" in favor or using $this in the context.

I'v just submited the PR #3273 that fixes this issue.

Going to close this in favor of the PR.

Thanks again for the bug report,
Pedro

Was this page helpful?
0 / 5 - 0 ratings

Related issues

deepaksp picture deepaksp  路  3Comments

mklahorst picture mklahorst  路  3Comments

M0H3N picture M0H3N  路  3Comments

lotarbo picture lotarbo  路  3Comments

jorgepires picture jorgepires  路  3Comments