[4.x] Ensure User::hasRole() checks group roles too

[ryanmitchell]

I mentioned this in the comments of #6131 and forgot to loop back to it.

When checking if a user has a role ($user->hasRole('x')), should it not also check the user's group for that role too, seeing as groups can also be associated with roles. Otherwise the burden is on the developer to check for this and it may not be apparent to most people.

I see you are merging group permissions with user permissions when checking for hasPermission(), so this feels like a natural evolution of that.

As always, please push back on this if I've misunderstood the intention.

[SteveEdson]

FWIW, I think this makes sense, but could it be worth putting it behind a 2nd optional argument so it's less of a unexpected/breaking change? Something like:

->hasRole($role, $includeGroups = false) {
  ...
}

[jasonvarga]

The argument is a good idea. Or maybe we just put this into v5.

The original reason this didn't fall back to groups was because if we do it your way, there's no longer a way to see if a user has an explicitly set role.

Typically you shouldn't really be mixing and matching assigning roles directly to users, and groups. But I know it's simple to do that so I'm sure many people have.

Also if you're blocking off parts of your site, you should probably be using "can" to check for permission rather than checking if they have a specific role. That's why the merging happens within permissions.

If we add an argument (or a separate method) maybe we could make it clearer when opting into certain things.

e.g. when filtering users by role in the cp, maybe there's an additional [ ] inherit from groups checkbox in the filter.
or in the user:is tag, we could have a inherit_from_groups="true" parameter.

[ryanmitchell]

Yep thats all fair - I expected the push back. One to keep on the radar.

I would expect (coming from the same behaviour in other Laravel systems) that it would check groups for the role too.