[4.x] Fix 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.
Fixes: #2498.
Fixes: #7578

[jasonvarga]

Made a few change requests in individual comments. Happy for pushback.

Not sure how this fits in with Eloquent

This whole group query thing was clearly not very well thought out. 😞
Probably ideally we'd be able to treat it more like a relationship somehow. Or maybe do a similar thing to entries and the whereTaxonomy method. We could get the user ids in the appropriate groups then have a whereIn('id', all the ids from the groups)

[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.

[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.

[eiiot]

Anyone know when this will be released?

[edalzell]

It was released a while back

[duncanmcclean]

Anyone know when this will be released?

This was released as part of v4.46.0.