Refactor groups service and controller again

_ide_helper.php

@@ -17317,119 +17317,30 @@
      */ 
         class Groups {
                     /**
-         * Sets the user on behalf of whom the service act
+         * Assign one or more accounts to a group
          *
+         * @param array|int $ids accounts ids to assign
          * @param \App\Models\User $user
-         * @return self 
-         * @static 
-         */ 
-        public static function for($user)
-        {
-                        /** @var \App\Services\GroupService $instance */
-                        return $instance->for($user);
-        }
-                    /**
-         * Sets the service to return group collections prepended with the 'All' pseudo group
-         *
-         * @return self 
-         * @static 
-         */ 
-        public static function withTheAllGroup()
-        {
-                        /** @var \App\Services\GroupService $instance */
-                        return $instance->withTheAllGroup();
-        }
-                    /**
-         * Get one or multiple group by their primary keys
-         *
-         * @param int|array $ids
-         * @return \App\Services\Collection<int, Group>|Group
-         * @throws \Illuminate\Database\Eloquent\ModelNotFoundException<\App\Models\Group>
-         * @static 
-         */ 
-        public static function get($ids)
-        {
-                        /** @var \App\Services\GroupService $instance */
-                        return $instance->get($ids);
-        }
-                    /**
-         * Returns all existing groups preprended with the 'All' group for the given user
-         *
-         * @return \App\Services\Collection<int, Group>
-         * @static 
-         */ 
-        public static function all()
-        {
-                        /** @var \App\Services\GroupService $instance */
-                        return $instance->all();
-        }
-                    /**
-         * Returns all accounts of the group
-         *
-         * @param \App\Models\Group $group
-         * @return \App\Services\Collection<int, \App\Models\TwoFAccount>
-         * @static 
-         */ 
-        public static function accounts($group)
-        {
-                        /** @var \App\Services\GroupService $instance */
-                        return $instance->accounts($group);
-        }
-                    /**
-         * Creates a group
-         *
-         * @param array $data
-         * @return \App\Models\Group The created group
-         * @throws \Illuminate\Auth\Access\AuthorizationException
-         * @throws \Exception
-         * @static 
-         */ 
-        public static function create($data)
-        {
-                        /** @var \App\Services\GroupService $instance */
-                        return $instance->create($data);
-        }
-                    /**
-         * Updates a group using a list of values
-         *
-         * @param \App\Models\Group $group The group
-         * @param array $data The parameters
-         * @return \App\Models\Group The updated group
+         * @param \App\Models\Group|null $group The group the accounts will be assigned to
+         * @return void 
          * @throws \Illuminate\Auth\Access\AuthorizationException
-         * @throws \Exception
          * @static 
          */ 
-        public static function update($group, $data)
+        public static function assign($ids, $user, $group = null)
         {
-                        /** @var \App\Services\GroupService $instance */
-                        return $instance->update($group, $data);
+                        \App\Services\GroupService::assign($ids, $user, $group);
         }
                     /**
-         * Deletes one or more groups
+         * Prepends the pseudo group named 'All' to a group collection
          *
-         * @param int|array $ids group ids to delete
-         * @return int The number of deleted
-         * @static 
-         */ 
-        public static function delete($ids)
-        {
-                        /** @var \App\Services\GroupService $instance */
-                        return $instance->delete($ids);
-        }
-                    /**
-         * Assign one or more accounts to a group
-         *
-         * @param array|int $ids accounts ids to assign
-         * @param \App\Models\Group $group The target group
-         * @return void 
-         * @throws \Illuminate\Auth\Access\AuthorizationException
-         * @throws \Illuminate\Database\Eloquent\ModelNotFoundException<\App\Models\TwoFAccount>
+         * @param \App\Services\Collection<int,  Group>  $groups
+         * @param \App\Models\User $user
+         * @return \App\Services\Collection<int, Group>
          * @static 
          */ 
-        public static function assign($ids, $group = null)
+        public static function prependTheAllGroup($groups, $user)
         {
-                        /** @var \App\Services\GroupService $instance */
-                        $instance->assign($ids, $group);
+                        return \App\Services\GroupService::prependTheAllGroup($groups, $user);
         }
          
     }

app/Api/v1/Controllers/GroupController.php

@@ -14,15 +14,18 @@ use Illuminate\Http\Request;
 class GroupController extends Controller
 {
     /**
-     * Display a listing of the resource.
+     * Display all user groups.
      *
+     * @param  \Illuminate\Http\Request  $request
      * @return \Illuminate\Http\Resources\Json\AnonymousResourceCollection
      */
     public function index(Request $request)
     {
-        $groups = Groups::for($request->user())->withTheAllGroup()->all();
+        // We do not use fluent call all over the call chain to ease tests
+        $user = $request->user();
+        $groups = $user->groups()->withCount('twofaccounts')->get();
 
-        return GroupResource::collection($groups);
+        return GroupResource::collection(Groups::prependTheAllGroup($groups, $request->user()));
     }
 
     /**
@@ -33,9 +36,11 @@ class GroupController extends Controller
      */
     public function store(GroupStoreRequest $request)
     {
+        $this->authorize('create', Group::class);
+
         $validated = $request->validated();
 
-        $group = Groups::for($request->user())->create($validated);
+        $group = $request->user()->groups()->create($validated);
 
         return (new GroupResource($group))
             ->response()
@@ -46,12 +51,11 @@ class GroupController extends Controller
      * Display the specified resource.
      *
      * @param  \App\Models\Group  $group
-     * @param  \Illuminate\Http\Request  $request
      * @return \App\Api\v1\Resources\GroupResource
      */
-    public function show(Group $group, Request $request)
+    public function show(Group $group)
     {
-        $group = Groups::for($request->user())->get($group->id);
+        $this->authorize('view', $group);
 
         return new GroupResource($group);
     }
@@ -65,9 +69,11 @@ class GroupController extends Controller
      */
     public function update(GroupStoreRequest $request, Group $group)
     {
+        $this->authorize('update', $group);
+
         $validated = $request->validated();
 
-        Groups::for($request->user())->update($group, $validated);
+        $group->update($validated);
 
         return new GroupResource($group);
     }
@@ -81,9 +87,11 @@ class GroupController extends Controller
      */
     public function assignAccounts(GroupAssignRequest $request, Group $group)
     {
+        $this->authorize('update', $group);
+
         $validated = $request->validated();
 
-        Groups::for($request->user())->assign($validated['ids'], $group);
+        Groups::assign($validated['ids'], $request->user(), $group);
 
         return new GroupResource($group);
     }
@@ -92,26 +100,26 @@ class GroupController extends Controller
      * Get accounts assigned to the group
      *
      * @param  \App\Models\Group  $group
-     * @param  \Illuminate\Http\Request  $request
      * @return \App\Api\v1\Resources\TwoFAccountCollection
      */
-    public function accounts(Group $group, Request $request)
+    public function accounts(Group $group)
     {
-        $groups = Groups::for($request->user())->accounts($group);
+        $this->authorize('view', $group);
 
-        return new TwoFAccountCollection($groups);
+        return new TwoFAccountCollection($group->twofaccounts);
     }
 
     /**
      * Remove the specified resource from storage.
      *
      * @param  \App\Models\Group  $group
-     * @param  \Illuminate\Http\Request  $request
      * @return \Illuminate\Http\JsonResponse
      */
-    public function destroy(Group $group, Request $request)
+    public function destroy(Group $group)
     {
-        Groups::for($request->user())->delete($group->id);
+        $this->authorize('delete', $group);
+
+        $group->delete();
 
         return response()->json(null, 204);
     }

app/Models/Group.php

@@ -74,19 +74,13 @@ class Group extends Model
         parent::boot();
 
         static::created(function (object $model) {
-            // @codeCoverageIgnoreStart
-            Log::info(sprintf('Group %s (id #%d) created ', var_export($model->name, true), $model->id));
-            // @codeCoverageIgnoreEnd
+            Log::info(sprintf('Group %s (id #%d) created for user ID #%s', var_export($model->name, true), $model->id, $model->user_id));
         });
         static::updated(function (object $model) {
-            // @codeCoverageIgnoreStart
-            Log::info(sprintf('Group %s (id #%d) updated ', var_export($model->name, true), $model->id));
-            // @codeCoverageIgnoreEnd
+            Log::info(sprintf('Group %s (id #%d) updated by user ID #%s', var_export($model->name, true), $model->id, $model->user_id));
         });
         static::deleted(function (object $model) {
-            // @codeCoverageIgnoreStart
             Log::info(sprintf('Group %s (id #%d) deleted ', var_export($model->name, true), $model->id));
-            // @codeCoverageIgnoreEnd
         });
     }
 

app/Policies/GroupPolicy.php

@@ -5,6 +5,8 @@ namespace App\Policies;
 use App\Models\Group;
 use App\Models\User;
 use Illuminate\Auth\Access\HandlesAuthorization;
+use Illuminate\Auth\Access\Response;
+use Illuminate\Support\Facades\Log;
 
 class GroupPolicy
 {
@@ -30,7 +32,13 @@ class GroupPolicy
      */
     public function view(User $user, Group $group)
     {
-        return $this->isOwnerOf($user, $group);
+        $can = $this->isOwnerOf($user, $group);
+
+        if (! $can) {
+            Log::notice(sprintf('User ID #%s cannot view group %s (ID #%s)', $user->id, var_export($group->name, true), $group->id));
+        }
+
+        return $can;
     }
 
     /**
@@ -43,7 +51,16 @@ class GroupPolicy
      */
     public function viewEach(User $user, Group $group, $groups)
     {
-        return $this->isOwnerOfEach($user, $groups);
+        $can = $this->isOwnerOfEach($user, $groups);
+
+        if (! $can) {
+            $ids = $groups->map(function ($group, $key) {
+                return $group->id;
+            });
+            Log::notice(sprintf('User ID #%s cannot view all groups in IDs #%s', $user->id, implode(',', $ids->toArray())));
+        }
+
+        return $can;
     }
 
     /**
@@ -54,6 +71,8 @@ class GroupPolicy
      */
     public function create(User $user)
     {
+        // Log::notice(sprintf('User ID #%s cannot create groups', $user->id));
+
         return true;
     }
 
@@ -66,7 +85,13 @@ class GroupPolicy
      */
     public function update(User $user, Group $group)
     {
-        return $this->isOwnerOf($user, $group);
+        $can = $this->isOwnerOf($user, $group);
+
+        if (! $can) {
+            Log::notice(sprintf('User ID #%s cannot update group %s (ID #%s)', $user->id, var_export($group->name, true), $group->id));
+        }
+
+        return $can;
     }
 
     /**
@@ -79,7 +104,16 @@ class GroupPolicy
      */
     public function updateEach(User $user, Group $group, $groups)
     {
-        return $this->isOwnerOfEach($user, $groups);
+        $can = $this->isOwnerOfEach($user, $groups);
+
+        if (! $can) {
+            $ids = $groups->map(function ($group, $key) {
+                return $group->id;
+            });
+            Log::notice(sprintf('User ID #%s cannot update all groups in IDs #%s', $user->id, implode(',', $ids->toArray())));
+        }
+
+        return $can;
     }
 
     /**
@@ -91,7 +125,13 @@ class GroupPolicy
      */
     public function delete(User $user, Group $group)
     {
-        return $this->isOwnerOf($user, $group);
+        $can = $this->isOwnerOf($user, $group);
+
+        if (! $can) {
+            Log::notice(sprintf('User ID #%s cannot delete group %s (ID #%s)', $user->id, var_export($group->name, true), $group->id));
+        }
+
+        return $can;
     }
 
     /**
@@ -104,7 +144,16 @@ class GroupPolicy
      */
     public function deleteEach(User $user, Group $group, $groups)
     {
-        return $this->isOwnerOfEach($user, $groups);
+        $can = $this->isOwnerOfEach($user, $groups);
+
+        if (! $can) {
+            $ids = $groups->map(function ($group, $key) {
+                return $group->id;
+            });
+            Log::notice(sprintf('User ID #%s cannot delete all groups in IDs #%s', $user->id, implode(',', $ids->toArray())));
+        }
+
+        return $can;
     }
 
     /**

app/Services/GroupService.php

@@ -6,287 +6,58 @@ use App\Models\Group;
 use App\Models\TwoFAccount;
 use App\Models\User;
 use Illuminate\Auth\Access\AuthorizationException;
-use Illuminate\Contracts\Support\Arrayable;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Support\Facades\Log;
 
 class GroupService
 {
     /**
-     * @var  \App\Models\User|null
-     */
-    protected $user;
-
-    /**
-     * @var  bool
-     */
-    protected $withTheAllGroup = false;
-
-
-    /**
-     * Create a new Group service instance.
+     * Assign one or more accounts to a group
      *
-     * @return void
-     */
-    public function __construct()
-    {
-        $this->user = null;
-    }
-
-    /**
-     * Sets the user on behalf of whom the service act
-     * 
+     * @param  array|int  $ids accounts ids to assign
      * @param  \App\Models\User  $user
-     * @return self
-     */
-    public function for(User $user)
-    {
-        $this->user = $user;
-
-        return $this;
-    }
-
-    /**
-     * Sets the service to return group collections prepended with the 'All' pseudo group
-     * 
-     * @return self
-     */
-    public function withTheAllGroup()
-    {
-        $this->withTheAllGroup = true;
-
-        return $this;
-    }
-
-    /**
-     * Get one or multiple group by their primary keys
-     *
-     * @param  int|array  $ids
-     * @return Collection<int, Group>|Group
+     * @param  \App\Models\Group|null  $group The group the accounts will be assigned to
+     * @return void
      * 
-     * @throws \Illuminate\Database\Eloquent\ModelNotFoundException<\App\Models\Group>
-     */
-    public function get(mixed $ids)
-    {
-        /**
-         * @var Collection<int, Group>|Group
-         */
-        $groups = Group::withCount('twofaccounts')->findOrFail($ids);
-
-        if ($groups instanceof Collection) {
-            if (! is_null($this->user)) {
-                // Authorization check
-                if ($this->user->cannot('viewEach', [(new Group), $groups])) {
-                    Log::notice(sprintf('User ID #%s cannot view all groups in IDs #%s', $this->user->id, implode(',', $ids)));
-                    throw new AuthorizationException();
-                }
-            }
-        }
-        else {
-            if (! is_null($this->user)) {
-                // Authorization check
-                if ($this->user->cannot('view', $groups)) {
-                    Log::notice(sprintf('User ID #%s cannot view group %s (#%s)', $this->user->id, var_export($groups->name, true), $groups->id));
-                    throw new AuthorizationException();
-                }
-            }
-        }
-
-        return $groups;
-    }
-
-    /**
-     * Returns all existing groups preprended with the 'All' group for the given user
-     *
-     * @return Collection<int, Group>
-     */
-    public function all() : Collection
-    {
-        $groups = ! is_null($this->user)
-            ? $this->user->groups()->withCount('twofaccounts')->get()
-            : Group::withCount('twofaccounts')->get();
-
-        return $this->withTheAllGroup
-            ? self::prependTheAllGroup($groups)
-            : $groups;
-    }
-
-    /**
-     * Returns all accounts of the group
-     *
-     * @param  \App\Models\Group  $group
-     * @return Collection<int, \App\Models\TwoFAccount>
-     */
-    public function accounts(Group $group) : Collection
-    {
-        if (! is_null($this->user)) {
-            // Authorization check
-            if ($this->user->cannot('view', $group)) {
-                Log::notice(sprintf('User ID #%s cannot view group ID #%s', $this->user->id, $group->id));
-                throw new AuthorizationException();
-            }
-        }
-
-        return $group->twofaccounts;
-    }
-
-    /**
-     * Creates a group
-     *
-     * @param  array  $data
-     * @return \App\Models\Group The created group
-     *
      * @throws \Illuminate\Auth\Access\AuthorizationException
-     * @throws \Exception
      */
-    public function create(array $data) : Group
+    public static function assign($ids, User $user, Group $group = null) : void
     {
-        if (! is_null($this->user)) {
-            // Authorization check
-            if ($this->user->cannot('create', Group::class)) {
-                Log::notice(sprintf('User ID #%s cannot create groups', $this->user->id));
-                throw new AuthorizationException();
-            }
-
-            $group = $this->user->groups()->create([
-                'name' => $data['name'],
-            ]);
-
-            Log::info(sprintf('Group %s created for user ID #%s', var_export($group->name, true), $this->user->id));
-            
-            return $group;
-        }
-        else {
-            throw new \Exception('Cannot create a group without a user');
-        }
-    }
-
-    /**
-     * Updates a group using a list of values
-     *
-     * @param  \App\Models\Group  $group The group
-     * @param  array  $data The parameters
-     * @return \App\Models\Group The updated group
-     *
-     *
-     * @throws \Illuminate\Auth\Access\AuthorizationException
-     * @throws \Exception
-     */
-    public function update(Group $group, array $data) : Group
-    {
-        if (! is_null($this->user)) {
-            // Authorization check
-            if ($this->user->cannot('update', $group)) {
-                Log::notice(sprintf('User ID #%s cannot update group %s', $this->user->id, var_export($group->name, true)));
-                throw new AuthorizationException();
-            }
-
-            $group->update([
-                'name' => $data['name'],
-            ]);
-    
-            Log::info(sprintf('Group %s updated by user ID #%s', var_export($group->name, true), $this->user->id));
-    
-            return $group;
-        }
-        else {
-            throw new \Exception('Cannot update a group without a user');
+        if (!$group) {
+            $group = self::defaultGroup($user);
         }
-    }
 
-    /**
-     * Deletes one or more groups
-     *
-     * @param  int|array  $ids group ids to delete
-     * @return int The number of deleted
-     */
-    public function delete($ids) : int
-    {
-        $ids = is_array($ids) ? $ids : [$ids];
-        $groups = Group::findMany($ids);
+        if ($group) {
+            $ids = is_array($ids) ? $ids : [$ids];
+            $twofaccounts = TwoFAccount::find($ids);
 
-        if ($groups->count() > 0) {
-            if (! is_null($this->user)) {
-                // Authorization check
-                if ($this->user->cannot('deleteEach', [$groups[0], $groups])) {
-                    Log::notice(sprintf('User ID #%s cannot delete all groups in IDs #%s', $this->user->id, implode(',', $ids)));
-                    throw new AuthorizationException();
-                }
+            if ($user->cannot('updateEach', [(new TwoFAccount), $twofaccounts])) {
+                throw new AuthorizationException();
             }
 
-            return Group::destroy($ids);
-        }
-
-        return 0;
-    }
-
-    /**
-     * Assign one or more accounts to a group
-     *
-     * @param  array|int  $ids accounts ids to assign
-     * @param  \App\Models\Group  $group The target group
-     * @return void
-     *
-     * @throws \Illuminate\Auth\Access\AuthorizationException
-     * @throws \Illuminate\Database\Eloquent\ModelNotFoundException<\App\Models\TwoFAccount>
-     */
-    public function assign($ids, Group $group = null) : void
-    {
-        $ids = is_array($ids) ? $ids : [$ids];
-        $twofaccounts = TwoFAccount::findOrFail($ids);
-
-        if (! is_null($this->user)) {
-            $group = $group ?? self::defaultGroup($this->user);
-
-            if ($group) {
-                // Authorization check on group
-                if ($this->user->cannot('update', $group)) {
-                    Log::notice(sprintf('User ID #%s cannot assign twofaccounts to group ID #%s', $this->user->id, $group->id));
-                    throw new AuthorizationException();
-                }
-
-                // Authorization check on twofaccounts
-                if ($this->user->cannot('updateEach', [$twofaccounts[0], $twofaccounts])) {
-                    Log::notice(sprintf('User ID #%s cannot assign twofaccounts IDs #%s to a group', $this->user->id, implode(',', $ids)));
-                    throw new AuthorizationException();
-                }
-
-                $group->twofaccounts()->saveMany($twofaccounts);
-                $group->loadCount('twofaccounts');
-    
-                Log::info(sprintf('Twofaccounts IDs #%s assigned to group %s (id #%s)', implode(',', $ids), var_export($group->name, true), $group->id));
-            } else {
-                Log::info(sprintf('Cannot find a group to assign the TwoFAccounts IDs #%s to', implode(',', $ids)));
-            }
-        }
-        else if ($group) {
             $group->twofaccounts()->saveMany($twofaccounts);
             $group->loadCount('twofaccounts');
 
-            Log::info(sprintf('Twofaccounts IDs #%s assigned to group %s (id #%s)', implode(',', $ids), var_export($group->name, true), $group->id));
-        }
-        else
-        {
-            Log::info(sprintf('No group to assign the TwoFAccounts IDs #%s to', implode(',', $ids)));
+            Log::info(sprintf('Twofaccounts #%s assigned to group %s (ID #%s)', implode(',', $ids), var_export($group->name, true), $group->id));
         }
+        else Log::info('Cannot find a group to assign the TwoFAccounts to');
     }
 
     /**
      * Prepends the pseudo group named 'All' to a group collection
      *
      * @param  Collection<int, Group>  $groups
+     * @param  \App\Models\User  $user
      * @return Collection<int, Group>
      */
-    private function prependTheAllGroup(Collection $groups) : Collection
+    public static function prependTheAllGroup(Collection $groups, User $user) : Collection
     {
         $theAllGroup = new Group([
             'name' => __('commons.all'),
         ]);
 
         $theAllGroup->id                 = 0;
-        $theAllGroup->twofaccounts_count = is_null($this->user)
-                                                ? TwoFAccount::count()
-                                                : TwoFAccount::where('user_id', $this->user->id)->count();
+        $theAllGroup->twofaccounts_count = $user->twofaccounts->count();
 
         return $groups->prepend($theAllGroup);
     }

tests/Api/v1/Controllers/GroupControllerTest.php

@@ -16,7 +16,19 @@ class GroupControllerTest extends FeatureTestCase
     /**
      * @var \App\Models\User|\Illuminate\Contracts\Auth\Authenticatable
      */
-    protected $user;
+    protected $user, $anotherUser;
+
+    /**
+     * @var App\Models\Group
+     */
+    protected $userGroupA, $userGroupB, $anotherUserGroupA, $anotherUserGroupB;
+
+    /**
+     * @var App\Models\TwoFAccount
+     */
+    protected $twofaccountA, $twofaccountB, $twofaccountC, $twofaccountD;
+
+    private const NEW_GROUP_NAME = 'MyNewGroup';
 
     /**
      * @test
@@ -26,30 +38,52 @@ class GroupControllerTest extends FeatureTestCase
         parent::setUp();
 
         $this->user = User::factory()->create();
+        $this->userGroupA = Group::factory()->for($this->user)->create();
+        $this->userGroupB = Group::factory()->for($this->user)->create();
+
+        $this->twofaccountA = TwoFAccount::factory()->for($this->user)->create([
+            'group_id' => $this->userGroupA->id,
+        ]);
+        $this->twofaccountB = TwoFAccount::factory()->for($this->user)->create([
+            'group_id' => $this->userGroupA->id,
+        ]);
+
+        $this->anotherUser = User::factory()->create();
+        $this->anotherUserGroupA = Group::factory()->for($this->anotherUser)->create();
+        $this->anotherUserGroupB = Group::factory()->for($this->anotherUser)->create();
+
+        $this->twofaccountC = TwoFAccount::factory()->for($this->anotherUser)->create([
+            'group_id' => $this->anotherUserGroupA->id,
+        ]);
+        $this->twofaccountD = TwoFAccount::factory()->for($this->anotherUser)->create([
+            'group_id' => $this->anotherUserGroupB->id,
+        ]);
     }
 
     /**
      * @test
      */
-    public function test_index_returns_group_collection_with_pseudo_group()
+    public function test_index_returns_user_groups_only_with_pseudo_group()
     {
-        Group::factory()->count(3)->for($this->user)->create();
-
-        $response = $this->actingAs($this->user, 'api-guard')
+        $this->actingAs($this->user, 'api-guard')
             ->json('GET', '/api/v1/groups')
             ->assertOk()
-            ->assertJsonCount(4, $key = null)
-            ->assertJsonStructure([
-                '*' => [
-                    'id',
-                    'name',
-                    'twofaccounts_count',
+            ->assertExactJson([
+                '0' => [
+                    'id'                 => 0,
+                    'name'               => 'All',
+                    'twofaccounts_count' => 2,
+                ],
+                '1' => [
+                    'id'                 => $this->userGroupA->id,
+                    'name'               => $this->userGroupA->name,
+                    'twofaccounts_count' => 2,
+                ],
+                '2' => [
+                    'id'                 => $this->userGroupB->id,
+                    'name'               => $this->userGroupB->name,
+                    'twofaccounts_count' => 0,
                 ],
-            ])
-            ->assertJsonFragment([
-                'id'                 => 0,
-                'name'               => 'All',
-                'twofaccounts_count' => 0,
             ]);
     }
 
@@ -58,15 +92,20 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_store_returns_created_group_resource()
     {
-        $response = $this->actingAs($this->user, 'api-guard')
+        $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/groups', [
-                'name' => 'My second group',
+                'name' => self::NEW_GROUP_NAME,
             ])
             ->assertCreated()
             ->assertJsonFragment([
-                'name'               => 'My second group',
+                'name'               => self::NEW_GROUP_NAME,
                 'twofaccounts_count' => 0,
             ]);
+
+        $this->assertDatabaseHas('groups', [
+            'name'    => self::NEW_GROUP_NAME,
+            'user_id' => $this->user->id,
+        ]);
     }
 
     /**
@@ -74,13 +113,14 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_store_invalid_data_returns_validation_error()
     {
-        $response = $this->actingAs($this->user, 'api-guard')
+        $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/groups', [
                 'name' => null,
             ])
             ->assertStatus(422);
     }
 
+
     /**
      * @test
      */
@@ -112,6 +152,19 @@ class GroupControllerTest extends FeatureTestCase
             ]);
     }
 
+    /**
+     * @test
+     */
+    public function test_show_group_of_another_user_is_forbidden()
+    {
+        $response = $this->actingAs($this->anotherUser, 'api-guard')
+            ->json('GET', '/api/v1/groups/' . $this->userGroupA->id)
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
+            ]);
+    }
+
     /**
      * @test
      */
@@ -159,6 +212,21 @@ class GroupControllerTest extends FeatureTestCase
             ->assertStatus(422);
     }
 
+    /**
+     * @test
+     */
+    public function test_update_group_of_another_user_is_forbidden()
+    {
+        $response = $this->actingAs($this->anotherUser, 'api-guard')
+            ->json('PUT', '/api/v1/groups/' . $this->userGroupA->id, [
+                'name' => 'name updated',
+            ])
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
+            ]);
+    }
+
     /**
      * @test
      */
@@ -214,18 +282,40 @@ class GroupControllerTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_get_assigned_accounts_returns_twofaccounts_collection()
+    public function test_assign_to_group_of_another_user_is_forbidden()
     {
-        $group    = Group::factory()->for($this->user)->create();
-        $accounts = TwoFAccount::factory()->count(2)->for($this->user)->create();
+        $response = $this->actingAs($this->anotherUser, 'api-guard')
+            ->json('POST', '/api/v1/groups/' . $this->userGroupA->id . '/assign', [
+                'ids' => [$this->twofaccountC->id, $this->twofaccountD->id],
+            ])
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
+            ]);
+    }
 
-        $assign = $this->actingAs($this->user, 'api-guard')
-            ->json('POST', '/api/v1/groups/' . $group->id . '/assign', [
-                'ids' => [$accounts[0]->id, $accounts[1]->id],
+    /**
+     * @test
+     */
+    public function test_assign_accounts_of_another_user_is_forbidden()
+    {
+        $response = $this->actingAs($this->user, 'api-guard')
+            ->json('POST', '/api/v1/groups/' . $this->userGroupA->id . '/assign', [
+                'ids' => [$this->twofaccountC->id, $this->twofaccountD->id],
+            ])
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
             ]);
+    }
 
+    /**
+     * @test
+     */
+    public function test_accounts_returns_twofaccounts_collection()
+    {
         $response = $this->actingAs($this->user, 'api-guard')
-            ->json('GET', '/api/v1/groups/' . $group->id . '/twofaccounts')
+            ->json('GET', '/api/v1/groups/' . $this->userGroupA->id . '/twofaccounts')
             ->assertOk()
             ->assertJsonCount(2)
             ->assertJsonStructure([
@@ -240,24 +330,22 @@ class GroupControllerTest extends FeatureTestCase
                     'period',
                     'counter',
                 ],
+            ])
+            ->assertJsonFragment([
+                'account' => $this->twofaccountA->account,
+            ])
+            ->assertJsonFragment([
+                'account' => $this->twofaccountB->account,
             ]);
     }
 
     /**
      * @test
      */
-    public function test_get_assigned_accounts_returns_twofaccounts_collection_with_secret()
+    public function test_accounts_returns_twofaccounts_collection_with_secret()
     {
-        $group    = Group::factory()->for($this->user)->create();
-        $accounts = TwoFAccount::factory()->count(2)->for($this->user)->create();
-
-        $assign = $this->actingAs($this->user, 'api-guard')
-            ->json('POST', '/api/v1/groups/' . $group->id . '/assign', [
-                'ids' => [$accounts[0]->id, $accounts[1]->id],
-            ]);
-
         $response = $this->actingAs($this->user, 'api-guard')
-            ->json('GET', '/api/v1/groups/' . $group->id . '/twofaccounts?withSecret=1')
+            ->json('GET', '/api/v1/groups/' . $this->userGroupA->id . '/twofaccounts?withSecret=1')
             ->assertOk()
             ->assertJsonCount(2)
             ->assertJsonStructure([
@@ -279,7 +367,7 @@ class GroupControllerTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_get_assigned_accounts_of_missing_group_returns_not_found()
+    public function test_accounts_of_missing_group_returns_not_found()
     {
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('GET', '/api/v1/groups/1000/twofaccounts')
@@ -289,6 +377,19 @@ class GroupControllerTest extends FeatureTestCase
             ]);
     }
 
+    /**
+     * @test
+     */
+    public function test_accounts_of_another_user_group_is_forbidden()
+    {
+        $response = $this->actingAs($this->anotherUser, 'api-guard')
+            ->json('GET', '/api/v1/groups/' . $this->userGroupA->id . '/twofaccounts')
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
+            ]);
+    }
+
     /**
      * test Group deletion via API
      *
@@ -298,7 +399,7 @@ class GroupControllerTest extends FeatureTestCase
     {
         $group = Group::factory()->for($this->user)->create();
 
-        $response = $this->actingAs($this->user, 'api-guard')
+        $this->actingAs($this->user, 'api-guard')
             ->json('DELETE', '/api/v1/groups/' . $group->id)
             ->assertNoContent();
     }
@@ -310,11 +411,24 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_destroy_missing_group_returns_not_found()
     {
-        $response = $this->actingAs($this->user, 'api-guard')
+        $this->actingAs($this->user, 'api-guard')
             ->json('DELETE', '/api/v1/groups/1000')
             ->assertNotFound()
             ->assertJsonStructure([
                 'message',
             ]);
     }
+
+    /**
+     * @test
+     */
+    public function test_destroy_group_of_another_user_is_forbidden()
+    {
+        $response = $this->actingAs($this->anotherUser, 'api-guard')
+        ->json('DELETE', '/api/v1/groups/'.$this->userGroupA->id)
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
+            ]);
+    }
 }

tests/Feature/Services/GroupServiceTest.php

@@ -6,10 +6,7 @@ use App\Facades\Groups;
 use App\Models\Group;
 use App\Models\TwoFAccount;
 use App\Models\User;
-use App\Policies\GroupPolicy;
 use Illuminate\Auth\Access\AuthorizationException;
-use Illuminate\Database\Eloquent\Collection;
-use Mockery\MockInterface;
 use Tests\FeatureTestCase;
 
 /**
@@ -74,279 +71,6 @@ class GroupServiceTest extends FeatureTestCase
         $this->twofaccountThree = TwoFAccount::factory()->for($this->otherUser)->create([
             'group_id' => $this->groupThree->id,
         ]);
-        TwoFAccount::factory()->for($this->otherUser)->create([
-            'group_id' => $this->groupThree->id,
-        ]);
-    }
-
-    /**
-     * @test
-     */
-    public function test_get_a_user_group_returns_a_group()
-    {
-        $group = Groups::for($this->user)->get($this->twofaccountOne->id);
-
-        $this->assertInstanceOf(Group::class, $group);
-        $this->assertEquals($this->twofaccountOne->id, $group->id);
-    }
-
-    /**
-     * @test
-     */
-    public function test_get_multiple_user_group_returns_a_group_collection()
-    {
-        $groups = Groups::for($this->user)->get([$this->twofaccountOne->id, $this->twofaccountTwo->id]);
-
-        $this->assertInstanceOf(Collection::class, $groups);
-        $this->assertCount(2, $groups);
-        $this->assertEquals($this->twofaccountOne->id, $groups[0]->id);
-        $this->assertEquals($this->twofaccountTwo->id, $groups[1]->id);
-    }
-
-    /**
-     * @test
-     */
-    public function test_get_a_missing_group_returns_not_found()
-    {
-        $this->expectException(\Exception::class);
-
-        $group = Groups::get(1000);
-    }
-
-    /**
-     * @test
-     */
-    public function test_get_a_list_of_groups_with_a_missing_group_returns_not_found()
-    {
-        $this->expectException(\Exception::class);
-
-        $group = Groups::get([$this->twofaccountOne->id, 1000]);
-    }
-
-    /**
-     * @test
-     */
-    public function test_user_authorization_to_get()
-    {
-        $this->expectException(AuthorizationException::class);
-
-        Groups::for($this->otherUser)->get($this->twofaccountOne->id);
-    }
-
-    /**
-     * @test
-     */
-    public function test_user_authorization_to_multiple_get()
-    {
-        $this->expectException(AuthorizationException::class);
-
-        Groups::for($this->otherUser)->get([$this->twofaccountOne->id, $this->twofaccountThree->id]);
-    }
-
-    /**
-     * @test
-     */
-    public function test_all_returns_user_groups_only()
-    {
-        $groups = Groups::for($this->user)->all();
-
-        $this->assertCount(2, $groups);
-    }
-
-    /**
-     * @test
-     */
-    public function test_all_returns_all_groups()
-    {
-        $groups = Groups::all();
-
-        $this->assertCount(5, $groups);
-    }
-
-    /**
-     * @test
-     */
-    public function test_withTheAllGroup_returns_pseudo_group_on_top_of_groups()
-    {
-        $groups = Groups::for($this->user)->withTheAllGroup()->all();
-
-        $this->assertCount(3, $groups);
-        $this->assertEquals(0, $groups->first()->id);
-        $this->assertEquals(__('commons.all'), $groups->first()->name);
-        $this->assertEquals($this->groupOne->user_id, $groups[1]->user_id);
-        $this->assertEquals($this->groupTwo->user_id, $groups[2]->user_id);
-    }
-
-    /**
-     * @test
-     */
-    public function test_withTheAllGroup_returns_user_groups_with_count()
-    {
-        $groups = Groups::for($this->user)->withTheAllGroup()->all();
-
-        $this->assertEquals(2, $groups->first()->twofaccounts_count);
-        $this->assertEquals(1, $groups[1]->twofaccounts_count);
-        $this->assertEquals(1, $groups[2]->twofaccounts_count);
-    }
-
-    /**
-     * @test
-     */
-    public function test_withTheAllGroup_returns_all_groups_with_count()
-    {
-        $groups = Groups::withTheAllGroup()->all();
-
-        $this->assertEquals(4, $groups->first()->twofaccounts_count);
-        $this->assertEquals(1, $groups[1]->twofaccounts_count);
-        $this->assertEquals(1, $groups[2]->twofaccounts_count);
-        $this->assertEquals(2, $groups[3]->twofaccounts_count);
-    }
-
-    /**
-     * @test
-     */
-    public function test_create_persists_and_returns_created_group()
-    {
-        $newGroup = Groups::for($this->user)->create(['name' => self::NEW_GROUP_NAME]);
-
-        $this->assertDatabaseHas('groups', [
-            'name'    => self::NEW_GROUP_NAME,
-            'user_id' => $this->user->id,
-        ]);
-        $this->assertInstanceOf(Group::class, $newGroup);
-        $this->assertEquals(self::NEW_GROUP_NAME, $newGroup->name);
-    }
-
-    /**
-     * @test
-     */
-    public function test_user_authorization_to_create()
-    {
-        $this->mock(GroupPolicy::class, function (MockInterface $groupPolicy) {
-            $groupPolicy->shouldReceive('create')
-                ->andReturn(false);
-        });
-
-        $this->expectException(AuthorizationException::class);
-
-        Groups::for($this->user)->create(['name' => 'lorem'], $this->user);
-    }
-
-    /**
-     * @test
-     */
-    public function test_create_without_user_fails()
-    {
-        $this->expectException(\Exception::class);
-
-        Groups::create(['name' => 'lorem'], $this->user);
-    }
-
-    /**
-     * @test
-     */
-    public function test_update_persists_and_returns_updated_group()
-    {
-        $this->groupOne = Groups::for($this->user)->update($this->groupOne, ['name' => self::NEW_GROUP_NAME]);
-
-        $this->assertDatabaseHas('groups', ['name' => self::NEW_GROUP_NAME]);
-        $this->assertInstanceOf(Group::class, $this->groupOne);
-        $this->assertEquals(self::NEW_GROUP_NAME, $this->groupOne->name);
-    }
-
-    /**
-     * @test
-     */
-    public function test_user_authorization_to_update()
-    {
-        $this->expectException(AuthorizationException::class);
-
-        Groups::for($this->otherUser)->update($this->groupOne, ['name' => self::NEW_GROUP_NAME]);
-    }
-
-    /**
-     * @test
-     */
-    public function test_update_without_user_fails()
-    {
-        $this->expectException(\Exception::class);
-
-        Groups::update($this->groupOne, ['name' => self::NEW_GROUP_NAME]);
-    }
-
-    /**
-     * @test
-     */
-    public function test_delete_a_user_group_clears_db_and_returns_deleted_count()
-    {
-        $deleted = Groups::for($this->user)->delete($this->groupOne->id);
-
-        $this->assertDatabaseMissing('groups', ['id' => $this->groupOne->id]);
-        $this->assertEquals(1, $deleted);
-    }
-
-    /**
-     * @test
-     */
-    public function test_delete_multiple_user_groups_clears_db_and_returns_deleted_count()
-    {
-        $deleted = Groups::for($this->user)->delete([$this->groupOne->id, $this->groupTwo->id]);
-
-        $this->assertDatabaseMissing('groups', ['id' => $this->groupOne->id]);
-        $this->assertDatabaseMissing('groups', ['id' => $this->groupTwo->id]);
-        $this->assertEquals(2, $deleted);
-    }
-
-    /**
-     * @test
-     */
-    public function test_delete_missing_group_does_not_fail_and_returns_deleted_count()
-    {
-        $this->assertDatabaseMissing('groups', ['id' => 1000]);
-
-        $deleted = Groups::delete([$this->groupOne->id, 1000]);
-
-        $this->assertDatabaseMissing('groups', ['id' => $this->groupOne->id]);
-        $this->assertEquals(1, $deleted);
-    }
-
-    /**
-     * @test
-     */
-    public function test_delete_default_group_resets_defaultGroup_preference()
-    {
-        $this->user['preferences->defaultGroup'] = $this->groupOne->id;
-        $this->user->save();
-
-        Groups::delete($this->groupOne->id);
-
-        $this->user->refresh();
-        $this->assertEquals(0, $this->user->preferences['defaultGroup']);
-    }
-
-    /**
-     * @test
-     */
-    public function test_delete_active_group_resets_activeGroup_preference()
-    {
-        $this->user['preferences->rememberActiveGroup'] = true;
-        $this->user['preferences->activeGroup']         = $this->groupOne->id;
-        $this->user->save();
-
-        Groups::delete($this->groupOne->id, $this->user);
-
-        $this->user->refresh();
-        $this->assertEquals(0, $this->user->preferences['activeGroup']);
-    }
-
-    /**
-     * @test
-     */
-    public function test_user_authorization_to_delete()
-    {
-        $this->expectException(AuthorizationException::class);
-
-        Groups::for($this->otherUser)->delete($this->groupOne->id);
     }
 
     /**
@@ -354,20 +78,7 @@ class GroupServiceTest extends FeatureTestCase
      */
     public function test_assign_a_twofaccount_to_a_group_persists_the_relation()
     {
-        Groups::assign($this->twofaccountOne->id, $this->groupTwo);
-
-        $this->assertDatabaseHas('twofaccounts', [
-            'id'       => $this->twofaccountOne->id,
-            'group_id' => $this->groupTwo->id,
-        ]);
-    }
-
-    /**
-     * @test
-     */
-    public function test_assign_a_twofaccount_to_a_user_group_persists_the_relation()
-    {
-        Groups::for($this->user)->assign($this->twofaccountOne->id, $this->groupTwo);
+        Groups::assign($this->twofaccountOne->id, $this->user, $this->groupTwo);
 
         $this->assertDatabaseHas('twofaccounts', [
             'id'       => $this->twofaccountOne->id,
@@ -378,9 +89,9 @@ class GroupServiceTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_assign_multiple_twofaccounts_to_a_user_group_persists_the_relation()
+    public function test_assign_multiple_twofaccounts_to_group_persists_the_relation()
     {
-        Groups::for($this->user)->assign([$this->twofaccountOne->id, $this->twofaccountTwo->id], $this->groupTwo);
+        Groups::assign([$this->twofaccountOne->id, $this->twofaccountTwo->id], $this->user, $this->groupTwo);
 
         $this->assertDatabaseHas('twofaccounts', [
             'id'       => $this->twofaccountOne->id,
@@ -395,12 +106,12 @@ class GroupServiceTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_assign_a_twofaccount_to_no_group_assigns_to_default_group()
+    public function test_assign_a_twofaccount_to_no_group_assigns_to_user_default_group()
     {
         $this->user['preferences->defaultGroup'] = $this->groupTwo->id;
         $this->user->save();
 
-        Groups::for($this->user)->assign($this->twofaccountOne->id);
+        Groups::assign($this->twofaccountOne->id, $this->user);
 
         $this->assertDatabaseHas('twofaccounts', [
             'id'       => $this->twofaccountOne->id,
@@ -411,13 +122,13 @@ class GroupServiceTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_assign_a_twofaccount_to_no_group_assigns_to_active_group()
+    public function test_assign_a_twofaccount_to_no_group_assigns_to_user_active_group()
     {
         $this->user['preferences->defaultGroup'] = -1;
         $this->user['preferences->activeGroup']  = $this->groupTwo->id;
         $this->user->save();
 
-        Groups::for($this->user)->assign($this->twofaccountOne->id);
+        Groups::assign($this->twofaccountOne->id, $this->user);
 
         $this->assertDatabaseHas('twofaccounts', [
             'id'       => $this->twofaccountOne->id,
@@ -436,7 +147,7 @@ class GroupServiceTest extends FeatureTestCase
         $this->user['preferences->activeGroup']  = 1000;
         $this->user->save();
 
-        Groups::for($this->user)->assign($this->twofaccountOne->id);
+        Groups::assign($this->twofaccountOne->id, $this->user);
 
         $this->assertDatabaseHas('twofaccounts', [
             'id'       => $this->twofaccountOne->id,
@@ -447,40 +158,32 @@ class GroupServiceTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_user_authorization_to_assign_to_group()
+    public function test_user_can_assign_an_account()
     {
         $this->expectException(AuthorizationException::class);
 
-        Groups::for($this->otherUser)->assign($this->twofaccountOne->id, $this->otherUser->groups()->first());
+        Groups::assign($this->twofaccountThree->id, $this->user, $this->user->groups()->first());
     }
 
     /**
      * @test
      */
-    public function test_user_authorization_to_assign_multiple_to_group()
+    public function test_user_can_assign_multiple_accounts()
     {
         $this->expectException(AuthorizationException::class);
 
-        Groups::for($this->otherUser)->assign([$this->twofaccountOne->id, $this->otherUser->twofaccounts()->first()->id], $this->groupTwo);
+        Groups::assign([$this->twofaccountOne->id, $this->twofaccountThree->id], $this->user, $this->user->groups()->first());
     }
 
     /**
      * @test
      */
-    public function test_user_authorization_to_assign_an_account()
+    public function test_prependTheAllGroup_add_the_group_on_top_of_groups()
     {
-        $this->expectException(AuthorizationException::class);
-
-        Groups::for($this->user)->assign($this->twofaccountThree->id, $this->user->groups()->first());
-    }
-
-    /**
-     * @test
-     */
-    public function test_user_authorization_to_assign_multiple_accounts()
-    {
-        $this->expectException(AuthorizationException::class);
+        $groups = Groups::prependTheAllGroup($this->user->groups, $this->user);
 
-        Groups::for($this->user)->assign([$this->twofaccountOne->id, $this->twofaccountThree->id], $this->user->groups()->first());
+        $this->assertCount(3, $groups);
+        $this->assertEquals(0, $groups->first()->id);
+        $this->assertEquals(2, $groups->first()->twofaccounts_count);
     }
 }

tests/Unit/Api/v1/Controllers/GroupControllerTest.php

@@ -9,7 +9,6 @@ use App\Api\v1\Resources\GroupResource;
 use App\Api\v1\Resources\TwoFAccountReadResource;
 use App\Facades\Groups;
 use App\Models\Group;
-use App\Models\TwoFAccount;
 use App\Models\User;
 use Illuminate\Foundation\Testing\WithoutMiddleware;
 use Illuminate\Http\Request;
@@ -24,57 +23,47 @@ class GroupControllerTest extends TestCase
     use WithoutMiddleware;
 
     /**
-     * @var \App\Api\v1\Controllers\GroupController tested controller
-     */
-    protected $controller;
-
-    /**
-     * @var \App\Api\v1\Requests\GroupStoreRequest mocked request
-     */
-    protected $groupStoreRequest;
-
-    /**
-     * @var \App\Models\User mocked user
+     * @var \App\Models\User|\Illuminate\Contracts\Auth\Authenticatable
      */
     protected $user;
 
     /**
-     * @var \Illuminate\Http\Request mocked request
+     * 
      */
-    protected $request;
-
     public function setUp() : void
     {
         parent::setUp();
 
-        $this->groupStoreRequest = Mockery::mock(GroupStoreRequest::class);
-        $this->request           = Mockery::mock(Request::class);
-
-        $this->request->shouldReceive('user')
-            ->andReturn(new User());
-
-        $this->controller = new GroupController();
+        $this->user = new User();
+        
+        // We do not use $this->actingAs($this->user) to prevent intelephense
+        // static analysis error. Dumb, but I don't like errors...
+        $this->app['auth']->guard(null)->setUser($this->user);
+        $this->app['auth']->shouldUse(null);
     }
 
     /**
      * @test
      */
-    public function test_index_returns_api_resources_using_groupService()
+    public function test_index_returns_api_resources()
     {
-        $groups = Group::factory()->count(3)->make();
-
-        Groups::shouldReceive('for')
-            ->with($this->request->user())
+        $user       = Mockery::mock(User::class);
+        $request    = Mockery::mock(Request::class);
+        $groups     = Group::factory()->count(3)->make();
+        $controller = new GroupController();
+        
+        $user->shouldReceive('groups->withCount->get')
             ->once()
-            ->andReturnSelf()
-            ->shouldReceive('withTheAllGroup')
-            ->once()
-            ->andReturnSelf()
-            ->shouldReceive('all')
+            ->andReturn($groups);
+
+        $request->shouldReceive('user')
+            ->andReturn($user);
+
+        Groups::shouldReceive('prependTheAllGroup')
             ->once()
             ->andReturn($groups);
 
-        $response = $this->controller->index($this->request);
+        $response = $controller->index($request);
 
         $this->assertContainsOnlyInstancesOf(GroupResource::class, $response->collection);
     }
@@ -82,26 +71,19 @@ class GroupControllerTest extends TestCase
     /**
      * @test
      */
-    public function test_store_returns_api_resource_stored_using_groupService()
+    public function test_store_uses_validated_data_and_returns_api_resource()
     {
-        $group = Group::factory()->make();
-        $validated = ['name' => $group->name];
-
-        $this->groupStoreRequest->shouldReceive([
-            'validated' => $validated,
-            'user'      => new User(),
+        $request    = Mockery::mock(GroupStoreRequest::class);
+        $controller = new GroupController();
+        $group      = Group::factory()->for($this->user)->make();
+        $validated  = ['name' => $group->name];
+
+        $request->shouldReceive([
+            'validated'            => $validated,
+            'user->groups->create' => $group,
         ]);
 
-        Groups::shouldReceive('for')
-            ->with($this->groupStoreRequest->user())
-            ->once()
-            ->andReturnSelf()
-            ->shouldReceive('create')
-            ->with($validated)
-            ->once()
-            ->andReturn($group);
-
-        $response = $this->controller->store($this->groupStoreRequest);
+        $response = $controller->store($request);
 
         $this->assertInstanceOf(Group::class, $response->original);
         // $this->assertInstanceOf(GroupResource::class, $response);
@@ -110,20 +92,12 @@ class GroupControllerTest extends TestCase
     /**
      * @test
      */
-    public function test_show_returns_api_resource_using_groupService()
+    public function test_show_returns_api_resource()
     {
+        $controller = Mockery::mock(GroupController::class)->makePartial();
         $group = Group::factory()->make();
 
-        Groups::shouldReceive('for')
-            ->with($this->request->user())
-            ->once()
-            ->andReturnSelf()
-            ->shouldReceive('get')
-            ->with($group->id)
-            ->once()
-            ->andReturn($group);
-
-        $response = $this->controller->show($group, $this->request);
+        $response = $controller->show($group);
 
         $this->assertInstanceOf(GroupResource::class, $response);
     }
@@ -131,26 +105,18 @@ class GroupControllerTest extends TestCase
     /**
      * @test
      */
-    public function test_update_returns_api_resource_updated_using_groupService()
+    public function test_update_validates_data_and_returns_api_resource()
     {
-        $group = Group::factory()->make();
-        $validated = ['name' => $group->name];
+        $request    = Mockery::mock(GroupStoreRequest::class);
+        $controller = Mockery::mock(GroupController::class)->makePartial();
+        $group      = Group::factory()->make();
+        $validated  = ['name' => $group->name];
 
-        $this->groupStoreRequest->shouldReceive([
-                'validated' => $validated,
-                'user'      => new User(),
+        $request->shouldReceive([
+            'validated' => $validated,
         ]);
 
-        Groups::shouldReceive('for')
-            ->with($this->groupStoreRequest->user())
-            ->once()
-            ->andReturnSelf()
-            ->shouldReceive('update')
-            ->with($group, $validated)
-            ->once()
-            ->andReturn($group);
-
-        $response = $this->controller->update($this->groupStoreRequest, $group);
+        $response = $controller->update($request, $group);
 
         $this->assertInstanceOf(GroupResource::class, $response);
     }
@@ -160,24 +126,21 @@ class GroupControllerTest extends TestCase
      */
     public function test_assignAccounts_returns_api_resource_assigned_using_groupService()
     {
-        $group              = Group::factory()->make();
-        $groupAssignRequest = Mockery::mock(GroupAssignRequest::class);
-        $validated          = ['ids' => $group->id];
+        $request    = Mockery::mock(GroupAssignRequest::class);
+        $controller = Mockery::mock(GroupController::class)->makePartial();
+        $group      = Group::factory()->make();
+        $validated  = ['ids' => $group->id];
 
-        $groupAssignRequest->shouldReceive([
+        $request->shouldReceive([
             'validated' => $validated,
-            'user'      => new User(),
+            'user'      => $this->user,
         ]);
 
-        Groups::shouldReceive('for')
-            ->with($groupAssignRequest->user())
-            ->once()
-            ->andReturnSelf()
-            ->shouldReceive('assign')
-            ->with($group->id, $group)
+        Groups::shouldReceive('assign')
+            ->with($group->id, $this->user, $group)
             ->once();
 
-        $response = $this->controller->assignAccounts($groupAssignRequest, $group);
+        $response = $controller->assignAccounts($request, $group);
 
         $this->assertInstanceOf(GroupResource::class, $response);
     }
@@ -185,28 +148,12 @@ class GroupControllerTest extends TestCase
     /**
      * @test
      */
-    public function test_accounts_returns_api_resources_fetched_using_groupService()
+    public function test_accounts_returns_api_resources()
     {
-        $group = Group::factory()->make();
-
-        $groupAccounts = new TwoFAccount();
-        $groupAccounts = $groupAccounts->newCollection(
-            array(
-                new TwoFAccount(),
-                new TwoFAccount()
-            )
-        );
+        $controller = Mockery::mock(GroupController::class)->makePartial();
+        $group      = Group::factory()->make();
 
-        Groups::shouldReceive('for')
-            ->with($this->request->user())
-            ->once()
-            ->andReturnSelf()
-            ->shouldReceive('accounts')
-            ->with($group)
-            ->once()
-            ->andReturn($groupAccounts);
-
-        $response = $this->controller->accounts($group, $this->request);
+        $response = $controller->accounts($group);
 
         $this->assertContainsOnlyInstancesOf(TwoFAccountReadResource::class, $response->collection);
     }
@@ -216,20 +163,13 @@ class GroupControllerTest extends TestCase
      */
     public function test_destroy_uses_group_service()
     {
-        $group     = Group::factory()->make();
-        $group->id = 0;
-
-        Groups::shouldReceive('for')
-            ->with($this->request->user())
-            ->once()
-            ->andReturnSelf()
-            ->shouldReceive('delete')
-            ->with($group->id)
-            ->once()
-            ->andReturn(1);
+        $controller = Mockery::mock(GroupController::class)->makePartial();
+        $group      = Group::factory()->make();
+        $group->id  = 0;
 
-        $response = $this->controller->destroy($group, $this->request);
+        $response = $controller->destroy($group);
 
         $this->assertInstanceOf('Illuminate\Http\JsonResponse', $response);
+        $this->assertEquals(204, $response->status());
     }
 }