Fix stache user groups/roles querying

[ryanmitchell]

This PR updates the state store paths for user groups to use -> rather than / which resolves the errors in #6124 and follows up on the comments in #2498.

It also updates the getKeysWithWhere method in the UserQueryBuilder to have logic for roles and groups so that we do a comparison on their handle, rather than the entire object (which was failing). Not sure how this fits in with Eloquent... I might need to update this logic or handle it a different way?

Fixes: #6124 and #2498

[ryanmitchell]

Asides from the individual comment replies....

Bigger picture, while this works, the ->where('groups/{slug}') approach seems too stache-specific to me, I agree it would be better to try and aim for a whereIn('groups', [ids]), whereHas('groups', ...) or even a custom whereGroups() approach, so that its compatible with eloquent users. At the moment this tag wouldn't work at all with them (?).

It would need brought into the UserQueryBuilder so that it can be overridden by Eloquent.

Something similar also would need implemented for roles.

Do you have a preference?

[edit] A further question - should the User hasRole method not also be checking the user's groups for roles assigned?

[ryanmitchell]

@jasonvarga I've gone ahead and added whereRole, whereGroup etc to the query builder to be used for role and group querying.

It builds on the previous work so still uses the ->where('group/x') approach internally for file based users, but custom methods also then allow eloquent to use its own approach. You'll see I've added queries to the eloquent builder that should cover the common scenarios, but devs would be free to change these methods if they need something different.

[jmalko]

Please consider this PR - this documented functionality is broken in today's version of Statamic.

[edalzell]

Sorry, I retargeted onto 4.x, locally resolved the conflicts (perhaps incorrectly), but I'm getting an error now, Call to undefined method App\Models\User::roles().

Maybe the model is missing a relationship?

[jasonvarga]

@edalzell I resolved the merge and pushed up the change. I didn't verify that the PR works yet, though. Feel free to try it out now. Maybe your issue was an incorrect merge. This PR is over a year old (sorry 😬) so it might just need more tweaking.

[edalzell]

When I try a role filter (users in the DB), I get this error, with this patch:

[2023-07-06 10:08:06] local.ERROR: Call to undefined method App\Models\User::roles() {"userId":"cba0aed3-26f9-4ca9-a67f-8261a605d37b","exception":"[object] (BadMethodCallException(code: 0): Call to undefined method App\\Models\\User::roles() at /Users/erin/Sites/zakat/vendor/laravel/framework/src/Illuminate/Support/Traits/ForwardsCalls.php:67)
[stacktrace]
#0 /Users/erin/Sites/zakat/vendor/laravel/framework/src/Illuminate/Support/Traits/ForwardsCalls.php(36): Illuminate\\Database\\Eloquent\\Model::throwBadMethodCallException('roles')
#1 /Users/erin/Sites/zakat/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(2335): Illuminate\\Database\\Eloquent\\Model->forwardCallTo(Object(Illuminate\\Database\\Eloquent\\Builder), 'roles', Array)
#2 /Users/erin/Sites/zakat/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/QueriesRelationships.php(868): Illuminate\\Database\\Eloquent\\Model->__call('roles', Array)
#3 /Users/erin/Sites/zakat/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Relations/Relation.php(110): Illuminate\\Database\\Eloquent\\Builder->Illuminate\\Database\\Eloquent\\Concerns\\{closure}()
#4 /Users/erin/Sites/zakat/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/QueriesRelationships.php(867): Illuminate\\Database\\Eloquent\\Relations\\Relation::noConstraints(Object(Closure))
#5 /Users/erin/Sites/zakat/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/QueriesRelationships.php(39): Illuminate\\Database\\Eloquent\\Builder->getRelationWithoutConstraints('roles')
#6 /Users/erin/Sites/zakat/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/QueriesRelationships.php(152): Illuminate\\Database\\Eloquent\\Builder->has('roles', '>=', 1, 'and', Object(Closure))
#7 /Users/erin/Sites/zakat/vendor/statamic/cms/src/Query/EloquentQueryBuilder.php(41): Illuminate\\Database\\Eloquent\\Builder->whereHas('roles', Object(Closure))
#8 /Users/erin/Sites/zakat/vendor/statamic/cms/src/Auth/Eloquent/UserQueryBuilder.php(40): Statamic\\Query\\EloquentQueryBuilder->__call('whereHas', Array)
#9 /Users/erin/Sites/zakat/vendor/statamic/cms/src/Query/Scopes/Filters/UserRole.php(33): Statamic\\Auth\\Eloquent\\UserQueryBuilder->whereRole('field_rep')

[ryanmitchell]

Yes this assumes you define a roles and groups relation on your user model. That would need documented.

[edalzell]

Yes this assumes you define a roles and groups relation on your user model. That would need documented.

But how would that work? My roles and groups are flat file, not in the DB. Only my users are in the DB.

[ryanmitchell]

Hmm my memory is hazy on this and I’m not at my machine to dive into the code. I’ll take a fresh look once I’m back.

[ryanmitchell]

Ok @edalzell @jasonvarga I've updated this to not require a relation to be set. You'll see in the eloquent UserQueryBuilder it now checks if a relation exists, and if not it performs a whereExists on the role_user or group_user table.

This should cover off the default usage of people switching over from stache. If a dev wants to use a different approach for roles/groups (eg my PR) they would need to define the relationship on their user model.

[jasperjorna]

This one is quite breaking at the moment when using Eloquent for users. Can we expect this to be included in a hotfix release?

[jasonvarga]

It won't be a hotfix, it'll be a minor release.

[edalzell]

This one is quite breaking at the moment when using Eloquent for users. Can we expect this to be included in a hotfix release?

In the meantime you can composer patch it in to start using it.