[5.x] Changes to User role methods

src/Auth/Eloquent/User.php

@@ -4,8 +4,10 @@
 
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Support\Carbon;
+use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Hash;
 use Statamic\Auth\User as BaseUser;
+use Statamic\Contracts\Auth\Role as RoleContract;
 use Statamic\Data\ContainsSupplementalData;
 use Statamic\Facades\Role;
 use Statamic\Facades\UserGroup;
@@ -84,30 +86,28 @@ public function status()
         // TODO
     }
 
-    public function roles($roles = null)
+    public function roles(): Collection
     {
-        return is_null($roles)
-            ? $this->getRoles()
-            : $this->setRoles($roles);
+        return $this->explicitRoles()
+            ->merge($this->groups()->flatMap->roles()->keyBy->handle());
     }
 
-    protected function getRoles()
+    public function explicitRoles($roles = null)
     {
+        if (func_num_args() === 1) {
+            $this->roles = collect();
+
+            $this->assignRole($roles);
+
+            return $this;
+        }
+
         return $this->roles = $this->roles
             ?? (new Roles($this))->all()->map(function ($row) {
                 return Role::find($row->role_id);
             })->keyBy->handle();
     }
 
-    protected function setRoles($roles)
-    {
-        $this->roles = collect();
-
-        $this->assignRole($roles);
-
-        return $this;
-    }
-
     protected function saveRoles()
     {
         $roles = $this->roles()->map->id();
@@ -143,20 +143,20 @@ public function removeRole($role)
         return $this;
     }
 
-    public function hasRole($role)
-    {
-        return $this->roles()->has(
-            is_string($role) ? $role : $role->handle()
-        );
-    }
-
     public function groups($groups = null)
     {
         return is_null($groups)
             ? $this->getGroups()
             : $this->setGroups($groups);
     }
 
+    public function hasRole($role)
+    {
+        $role = $role instanceof RoleContract ? $role->handle() : $role;
+
+        return $this->roles()->has($role);
+    }
+
     protected function getGroups()
     {
         return $this->groups = $this->groups

src/Auth/File/User.php

@@ -3,10 +3,10 @@
 namespace Statamic\Auth\File;
 
 use Carbon\Carbon;
+use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Hash;
 use Statamic\Auth\PermissionCache;
 use Statamic\Auth\User as BaseUser;
-use Statamic\Contracts\Auth\Role as RoleContract;
 use Statamic\Contracts\Auth\UserGroup as UserGroupContract;
 use Statamic\Data\ContainsData;
 use Statamic\Data\Data;
@@ -151,16 +151,19 @@ public function getRememberTokenName()
         return 'remember_token';
     }
 
-    public function roles($roles = null)
+    public function roles(): Collection
     {
-        return is_null($roles)
-            ? $this->getRoles()
-            : $this->set('roles', $roles);
+        return $this->explicitRoles()
+            ->merge($this->groups()->flatMap->roles()->keyBy->handle());
     }
 
-    protected function getRoles()
+    public function explicitRoles($roles = null)
     {
-        return collect($this->get('roles', []))
+        if (func_num_args() === 1) {
+            return $this->set('roles', $roles);
+        }
+
+        return collect($this->get('roles'))
             ->map(function ($role) {
                 return Facades\Role::find($role);
             })->filter()->keyBy->handle();
@@ -193,13 +196,6 @@ public function removeRole($role)
         return $this;
     }
 
-    public function hasRole($role)
-    {
-        $role = $role instanceof RoleContract ? $role->handle() : $role;
-
-        return $this->roles()->has($role);
-    }
-
     public function addToGroup($group)
     {
         $groups = collect(Arr::wrap($group))->map(function ($group) {

src/Auth/User.php

@@ -13,6 +13,7 @@
 use Illuminate\Notifications\Notifiable;
 use Illuminate\Support\Facades\Password;
 use Statamic\Auth\Passwords\PasswordReset;
+use Statamic\Contracts\Auth\Role as RoleContract;
 use Statamic\Contracts\Auth\User as UserContract;
 use Statamic\Contracts\Data\Augmentable;
 use Statamic\Contracts\Data\Augmented;
@@ -147,6 +148,13 @@ public function getAuthPasswordName()
         return 'password';
     }
 
+    public function hasRole($role)
+    {
+        $role = $role instanceof RoleContract ? $role->handle() : $role;
+
+        return $this->roles()->has($role);
+    }
+
     /**
      * Get or set the blueprint.
      *

src/Console/Commands/ImportUsers.php

@@ -100,7 +100,7 @@ private function importUsers()
                 }
 
                 if (count($data->get('roles', [])) > 0) {
-                    $eloquentUser->roles($data->get('roles'));
+                    $eloquentUser->explicitRoles($data->get('roles'));
                 }
 
                 $eloquentUser->saveToDatabase();

src/Contracts/Auth/User.php

@@ -2,6 +2,8 @@
 
 namespace Statamic\Contracts\Auth;
 
+use Illuminate\Support\Collection;
+
 interface User
 {
     /**
@@ -20,7 +22,9 @@ public function email($email = null);
      */
     public function password($password = null);
 
-    public function roles();
+    public function roles(): Collection;
+
+    public function explicitRoles($roles = null);
 
     public function assignRole($role);
 

src/Http/Controllers/CP/Users/UsersController.php

@@ -179,7 +179,7 @@ public function store(Request $request)
             ->data($values);
 
         if ($request->roles && User::current()->can('assign roles')) {
-            $user->roles($request->roles);
+            $user->explicitRoles($request->roles);
         }
 
         if ($request->groups && User::current()->can('assign user groups')) {
@@ -295,7 +295,7 @@ public function update(Request $request, $user)
         $user->email($request->email);
 
         if (User::current()->can('assign roles')) {
-            $user->roles($request->roles);
+            $user->explicitRoles($request->roles);
         }
 
         if (User::current()->can('assign user groups')) {

src/Http/Controllers/UserController.php

@@ -84,7 +84,7 @@ public function register(Request $request)
             ->data($values);
 
         if ($roles = config('statamic.users.new_user_roles')) {
-            $user->roles($roles);
+            $user->explicitRoles($roles);
         }
 
         if ($groups = config('statamic.users.new_user_groups')) {

tests/Auth/FileUserTest.php

@@ -90,14 +90,14 @@ public function it_gets_permissions_from_a_cache()
 
         $userGroupRole->shouldReceive('id')->andReturn('usergrouprole');
         $userGroupRole->shouldReceive('handle')->andReturn('usergrouprole');
-        $userGroupRole->shouldReceive('permissions')->once()->andReturn(collect([
+        $userGroupRole->shouldReceive('permissions')->twice()->andReturn(collect([
             'permission one through user group',
             'permission two through user group',
         ]));
 
         $userGroup->shouldReceive('id')->andReturn('usergroup');
         $userGroup->shouldReceive('handle')->andReturn('usergroup');
-        $userGroup->shouldReceive('roles')->once()->andReturn(collect([$userGroupRole]));
+        $userGroup->shouldReceive('roles')->once()->andReturn(collect([$userGroupRole]))->times(4);
 
         Role::shouldReceive('find')->with('direct')->andReturn($directRole);
         Role::shouldReceive('all')->andReturn(collect([$directRole])); // the stache calls this when getting a user. unrelated to test.

tests/Preferences/PrecedenceTest.php

@@ -92,7 +92,7 @@ public function it_gives_precedence_to_role_order_assigned_on_user()
             'pleb' => Role::make()->permissions('super')->preferences(['alpha' => 'bar', 'charlie' => 'charlie']),
         ]);
 
-        $this->actingAs(User::make()->roles(['author', 'pleb']));
+        $this->actingAs(User::make()->explicitRoles(['author', 'pleb']));
 
         $expected = [
             'alpha' => 'foo', // This should be `foo`, because the `author` role is set first