Zizaco / entrust

Role-based Permissions for Laravel 5
MIT License
6.05k stars 1.29k forks source link

Deleting a user while failing a relation constraint removes the role_user record #954

Open bert-w opened 4 years ago

bert-w commented 4 years ago

Consider the following case:

I am going to delete a $user which has a relation to a record in the tickets table.

The tickets relation looks like:

`tickets`
user_id ON DELETE RESTRICT

This means that if I would delete the associated user (just regular $user->delete() through eloquent), the query should fail because of an "integrity constraint violation", which is what happens and is expected.

However, the role_user record of the user is deleted even though an exception is thrown because of the tickets relation. The role_user relation looks like this:

`role_user`
user_id ON DELETE CASCADE

The code in question seems to be this line: https://github.com/Zizaco/entrust/blob/master/src/Entrust/Traits/EntrustUserTrait.php#L91

This adds a static::deleting() to the user model, which in this case tries to remove all associated roles for this user, although that in itself should be logic related to the database (like InnoDB).

Note that a similar issue also occurs in: https://github.com/Zizaco/entrust/blob/master/src/Entrust/Traits/EntrustRoleTrait.php#L94 https://github.com/Zizaco/entrust/blob/master/src/Entrust/Traits/EntrustPermissionTrait.php#L32

I am assuming that this is a fix for MyISAM engines (since these dont support foreign keys if im correct ?). I personally am using InnoDB.

Solution 1: maybe change the event listeners from static::deleting to static::deleted so that on a transaction level it keeps functioning as well? I havent tested this change yet.

Solution 2: remove the event listeners if the engine is InnoDB, and ensure correct ON DELETE settings for the database tables (which seem to be correct https://github.com/Zizaco/entrust/blob/master/src/views/generators/migration.blade.php )