kiritokatklian / nova-permission

A Laravel Nova tool for Spatie's laravel-permission library
https://novapackages.com/packages/vyuldashev/nova-permission
MIT License
68 stars 15 forks source link

Set the Resource Model according to Spatie config #32

Closed meyer59 closed 3 months ago

meyer59 commented 3 months ago

Hi I am using custom Permission and Role models (i replaced them in the Spatie config). The reason i am using custom model isin order to Audit change with the laravel auditing package. It seems that the package have the Spatie model hard coded. Maybe in the Resource constructor we should get the model from the spatie config ? Thanks for your help here

kiritokatklian commented 3 months ago

@meyer59 the models are hard coded in the $model property, but there's a getModel() method that looks into the Spatie's PermissionRegistry class to get the actual model classes used by the Spatie package. I presume that'd be equal to the models specified in the config file. If that's not the case then the getModel() method isn't being used.

Edit: I opened PR #33. Can you please try it and let me know if it works, so I can merge it into master?

meyer59 commented 3 months ago

I saw the getModel() method in your Resource but i am not sure if Nova use it. Also I cannot see any reference to it in the Nova code or doc. I just tested your PR and it's working now. Thank you for your quick response

kiritokatklian commented 3 months ago

Indeed. Looks like remnant code from when vyuldashev, the original creator, added RoleBooleanGroup and PremissionBooleanGroup.

Glad to hear it works now. I tagged a new release 4.0.8

meyer59 commented 3 months ago

Hey You actually set the $model property with an instance of the permission class while Laravel Nova wants a class name so it doesn't work. (getPermissionClass()/getRoleClass() return an object of the class) Can you please fix the getModel() method to get a class name like so ?

public static function getModel()
    {
        return app(PermissionRegistrar::class)->getPermissionClass()::class;
       //or   return get_class(app(PermissionRegistrar::class)->getPermissionClass());
    }

Thanks

kiritokatklian commented 3 months ago

@meyer59 using ::class returns an error which is expected. getPermission/RoleClass is supposed to return a string, and not an object (see screenshot).

In what scenario is it not working as expected? I'd like to reproduce it if possible. It might be caused by another issue.

Screenshot 2024-05-18 at 22 27 42
kiritokatklian commented 3 months ago

I see it. It's an issue with the Role resource. Role::getModel is getting an object from the string returned by getRoleClass unlike Permission::getModel which returns getPermissionClass as is.

I refactored Role::getModel to work similar to the permission counterpart and added Role::getObject to do the job of the previous implementation. With that I'm not getting an error on the Roles index.

Fixed is tagged as 4.0.9. Let me know if that fixes the issue for you as well.

meyer59 commented 3 months ago

Hey When i update the version and visit Nova, i am getting an error Class "{"guard_name":"nova"}" not found. This is only with the v4.0.9. I don't know yet why this is poping up if you have any idea

meyer59 commented 3 months ago

I found it, the getModel() method from the your Role resource still returns an obect wich cannot be assigned to the $model nova property. Also please not that i am on the V5 of spatie permission.
Maybe you can just add a check if the value is an object return get_class() otherwise retun the value

kiritokatklian commented 3 months ago

Ahh, indeed in v5 it's returning an object and in v6 it's a string as one would expect given how the method is named.

Screenshot 2024-05-21 at 10 28 48

I can't downgrade to v5 to test due to other dependency constraints, but 4.0.10 with the check for object/string should work for v5 and below 🤞

meyer59 commented 3 months ago

Seems to work properly in v5. Thanks for your work here

kiritokatklian commented 3 months ago

Great! Thank you too for your help 😄