statamic / cms

The core Laravel CMS Composer package
https://statamic.com
Other
3.58k stars 490 forks source link

Database Role inserting (string) role as opposed to id of the role #9996

Closed icemancast closed 2 months ago

icemancast commented 2 months ago

Bug description

When I add user with a role other than super admin using our local database roles I get the error in the screenshot below.

I used a patch for this pr prior to it being imported and seemed to work: https://github.com/statamic/cms/pull/5686

I have since removed the patch and now have upgraded to the latest version as that pr was merged already. But looking at the insert error it seems it is trying to send the string of the role instead of the id that is associated with that role.

How to reproduce

Make sure you are using database roles and groups as opposed to statamic. Have the latest Statamic installed and add a user and attach a non super admin role to him.

Logs

No response

Environment

Environment
Application Name: Radix IoT
Laravel Version: 10.44.0
PHP Version: 8.2.18
Composer Version: 2.7.2
Environment: local
Debug Mode: ENABLED
URL: radixiot.test
Maintenance Mode: OFF

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: CACHED

Drivers
Broadcasting: log
Cache: statamic
Database: mysql
Logs: daily
Mail: mailgun
Queue: sync
Session: database

Statamic
Addons: 0
Antlers: runtime
Sites: 1
Stache Watcher: Enabled
Static Caching: Disabled
Version: 4.57.3 PRO

Installation

Other (please explain)

Antlers Parser

Runtime (default)

Additional details

This is a hybrid laravel and statamic application. The client already had users in the database and a dashboard that uses those user roles. Attached is the sql error I get.

Screenshot 2024-05-03 at 10 09 18 AM

icemancast commented 2 months ago

config/statamic/users.php

...
    'tables' => [
        'users' => 'users',
        'role_user' => 'role_user',
        'group_user' => 'group_user',
    ],
...
ryanmitchell commented 2 months ago

Thats how its meant to work - we use the role 'handle' rather than the 'id' as the linking key, to keep things consistent between database and file storage.

Seems like you just need to update your table to accept strings instead of integers.

icemancast commented 2 months ago

@ryanmitchell so alter role_id to be string? I guess I missed that in the upgrading of it.

ryanmitchell commented 2 months ago

Fairly sure its always been that way - but yep - see: https://github.com/statamic/cms/blob/f7575f65813feed8069c781af2edef9a792fea03/src/Console/Commands/stubs/auth/statamic_auth_tables.php.stub#L20

icemancast commented 2 months ago

Okay sorry last thing do I need to convert the current roles in that joiner table to their respective handles then too?

The roles was already created so I removed the statamics auth migration part and missed that. I would need to see how to update the application part to link that role via handle instead of id.

jasonvarga commented 2 months ago

I'm not sure why or how you ever ended up with role_user 's role_id column being an integer. 🤔 It's been a string since it was first created for v3 in 2019. https://github.com/statamic/cms/commit/c4dceb74bccc018063feb4aec8b83119e9989041#diff-7eb3c0413468774d8b7dbf18284b36a289cce3593f343bafa640f4359ad6dfe4R25

Even though the column is named role_id, we insert the handles.

icemancast commented 2 months ago

So thought I mentioned above but I didn’t that the roles were for the laravel application at first . The statamic side was added after.

jasonvarga commented 2 months ago

Oh yes you did, my bad. Makes sense now!

icemancast commented 1 month ago

Not opening again but this alters how this works on the laravel side now since i need a string to match the handle:

public function roles(): BelongsToMany
{
    return $this->belongsToMany(Role::class, 'role_user', 'user_id', 'role_id')->withTimestamps();
}

Should I just do a custom query? Do I miss any goodies for now using belongsToMany