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

New Relation::enforceMorphMap causes error when attaching resource via pivot #3547

Closed
dmason30 opened this issue Sep 13, 2021 · 9 comments
Closed

Comments

@dmason30
Copy link

dmason30 commented Sep 13, 2021

  • Laravel Version: 5.60.0
  • Nova Version: 3.29.0
  • PHP Version: 7.4
  • Database Driver & Version: MySql 8

TL;DR

Getting this exception on a belongsToMany relation via pivot after switching to Relation::enforceMorphMap as there isn't a mapping for Illuminate\Database\Eloquent\Relations\Pivot?!

Illuminate\Database\ClassMorphViolationException: No morph map defined for model 
[Illuminate\Database\Eloquent\Relations\Pivot]. 
in /vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasRelationships.php:736
Stack trace:
#0 /vendor/laravel/nova/src/Actions/ActionEvent.php(132): Illuminate\Database\Eloquent\Model->getMorphClass()
#1 /vendor/laravel/nova/src/Http/Controllers/ResourceAttachController.php(41): Laravel\Nova\Actions\ActionEvent::forAttachedResource(Object(Laravel\Nova\Http\Requests\NovaRequest), Object(App\Models\Admin), Object(Illuminate\Database\Eloquent\Relations\Pivot))
#2 /vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php(29): Laravel\Nova\Http\Controllers\ResourceAttachController->Laravel\Nova\Http\Controllers\{closure}(Object(Illuminate\Database\MySqlConnection))
#3 /vendor/laravel/framework/src/Illuminate/Database/DatabaseManager.php(388): Illuminate\Database\Connection->transaction(Object(Closure))
#4 /vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(261): Illuminate\Database\DatabaseManager->__call('transaction', Array)
#5 /vendor/laravel/nova/src/Http/Controllers/ResourceAttachController.php(46): Illuminate\Support\Facades\Facade::__callStatic('transaction', Array)

Description:

Laravel PR of change merged: laravel/framework#38656
Laravel version released in 8.59.0

Docs were changed to suggest only using this method:
https://laravel.com/docs/8.x/eloquent-relationships#custom-polymorphic-types

If you use the old Relation::morphMap this works fine still, however, to be strict about morph types it seems that it is now recommended to switch to Relation::enforceMorphMap

Detailed steps to reproduce the issue on a fresh Nova installation:

See the example structure and code below taken from laravel docs:
https://laravel.com/docs/8.x/eloquent-relationships#many-to-many

If you try and attach a role to the user in nova with this setup you will get the above error.

Structure

users
    id - integer
    name - string

roles
    id - integer
    name - string

role_user
    user_id - integer
    role_id - integer

Model

class User extends Model
{
    public function roles()
    {
        return $this->belongsToMany(Role::class);
    }
}

AppServiceProvider@register - Relation::enforceMorphMap:

Relation::enforceMorphMap([]);

App/Nova/UserResource@fields

BelongsToMany::make('Roles', 'roles', RoleResource::class)

Workaround

Add pivot classes to morph map:

Relation::enforceMorphMap([
    MorphPivot::class => MorphPivot::class,
    Pivot::class => Pivot::class,
]);

Note: I believe that a polymorphic pivot will have the same error with resolving the MorphPivot class and will need to have the same fix as this error.

Conclusion

It seems that the ActionEvent::forAttachedResource is getting Illuminate\Database\Eloquent\Relations\Pivot as the $pivot rather than a model and calls getMorphClass() on it which errors as it doesn't exist in the enforced morph map.

@crynobone
Copy link
Member

Belongs to many doesn't use morph mapping. Please provide correct example on a new issue so we can follow/replicate the issue properly.

@dmason30
Copy link
Author

@crynobone That wasn't the issue raised ActionEvent::forAttachedResource calls getMorphClass on the model regardless of it being polymorphic.

@crynobone crynobone reopened this Sep 13, 2021
@crynobone
Copy link
Member

To solve this you need to define using() and define every pivot models and register it via enforceMorphMap(). Laravel Nova can't do this automatically for you, which is why it using the default Pivot model.

@dmason30
Copy link
Author

dmason30 commented Sep 13, 2021

@crynobone Appreciate the suggestion and it would certainly work, however, we would have to create these useless model classes specifically for that purpose.

The issue there is that Laravel docs now has no mention of the old Relation::morphMap, I assume it has been deprecated. So most new comers are going to be using enforceMorphMap. I don't know if it just requires some docs update on laravel or nova to warn about this for belongsToMany.

The only fix would be some nova method to handle this which catches the getMorphClass exception and attempt to fallback to using the class reference only when the passed in $pivot is an instance of Pivot/MorphPivot. However, I understand you would probably not want to do this.

This workaround does work or disabling action events.

Relation::enforceMorphMap([
    MorphPivot::class => MorphPivot::class,
    Pivot::class => Pivot::class,
]);

@taylorotwell
Copy link
Member

The old method has NOT been deprecated.

@olivernybroe
Copy link

A simple solution in nova could be before the getMorphClass is called, the enforce state is saved in a temporary variable, then disabled and set back to the temporary variable.

This could also be added as a method to Relation called something like ignoreEnforceFor(Closure).

@crynobone
Copy link
Member

A simple solution in nova could be before the getMorphClass is called, the enforce state is saved in a temporary variable, then disabled and set back to the temporary variable.

This would cause an invalid state where on one side you wish to enforce morph map but on the other side, Laravel Nova hide/ignore this completely.

To solve this you need to define using() and define every pivot models and register it via enforceMorphMap(). Laravel Nova can't do this automatically for you, which is why it using the default Pivot model.

image

This works fine on Laravel Nova Dusk Suite application

@crynobone crynobone added the request Feature Request label Sep 14, 2021
@davidhemphill
Copy link
Contributor

👋 Hi there! In order to keep this repository focused on bug reports, we auto-close feature requests and requests for help. Feel free to post your feature requests so others can discuss and add reactions. We'll keep an eye on them for later planning.

@github-actions
Copy link

github-actions bot commented Oct 5, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 5, 2021
@crynobone crynobone removed the request Feature Request label Nov 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants