From 11b97bef0317839f84e8f331a1ba81be832ed201 Mon Sep 17 00:00:00 2001 From: erikn69 Date: Thu, 9 Nov 2023 10:19:08 -0500 Subject: [PATCH 1/3] Support strings as numeric ids --- src/Traits/HasPermissions.php | 6 +++--- src/Traits/HasRoles.php | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php index 2618b380..fa851e47 100644 --- a/src/Traits/HasPermissions.php +++ b/src/Traits/HasPermissions.php @@ -152,7 +152,7 @@ protected function convertToPermissionModels($permissions): array $permission = $permission->value; } - $method = is_int($permission) || PermissionRegistrar::isUid($permission) ? 'findById' : 'findByName'; + $method = is_numeric($permission) || PermissionRegistrar::isUid($permission) ? 'findById' : 'findByName'; return $this->getPermissionClass()::{$method}($permission, $this->getDefaultGuardName()); }, Arr::wrap($permissions)); @@ -172,7 +172,7 @@ public function filterPermission($permission, $guardName = null) $permission = $permission->value; } - if (is_int($permission) || PermissionRegistrar::isUid($permission)) { + if (is_numeric($permission) || PermissionRegistrar::isUid($permission)) { $permission = $this->getPermissionClass()::findById( $permission, $guardName ?? $this->getDefaultGuardName() @@ -480,7 +480,7 @@ protected function getStoredPermission($permissions) $permissions = $permissions->value; } - if (is_int($permissions) || PermissionRegistrar::isUid($permissions)) { + if (is_numeric($permissions) || PermissionRegistrar::isUid($permissions)) { return $this->getPermissionClass()::findById($permissions, $this->getDefaultGuardName()); } diff --git a/src/Traits/HasRoles.php b/src/Traits/HasRoles.php index f02f7c16..73ab3789 100644 --- a/src/Traits/HasRoles.php +++ b/src/Traits/HasRoles.php @@ -87,7 +87,7 @@ public function scopeRole(Builder $query, $roles, $guard = null, $without = fals $role = $role->value; } - $method = is_int($role) || PermissionRegistrar::isUid($role) ? 'findById' : 'findByName'; + $method = is_numeric($role) || PermissionRegistrar::isUid($role) ? 'findById' : 'findByName'; return $this->getRoleClass()::{$method}($role, $guard ?: $this->getDefaultGuardName()); }, Arr::wrap($roles)); @@ -229,7 +229,7 @@ public function hasRole($roles, string $guard = null): bool $roles = $roles->value; } - if (is_int($roles) || PermissionRegistrar::isUid($roles)) { + if (is_numeric($roles) || PermissionRegistrar::isUid($roles)) { $key = (new ($this->getRoleClass())())->getKeyName(); return $guard @@ -366,7 +366,7 @@ protected function getStoredRole($role): Role $role = $role->value; } - if (is_int($role) || PermissionRegistrar::isUid($role)) { + if (is_numeric($role) || PermissionRegistrar::isUid($role)) { return $this->getRoleClass()::findById($role, $this->getDefaultGuardName()); } From b3f23d4ee0f8d46090c26e7178901206a7f25072 Mon Sep 17 00:00:00 2001 From: erikn69 Date: Thu, 9 Nov 2023 16:36:13 -0500 Subject: [PATCH 2/3] Support strings as numeric ids removeRole, revokePermissionTo, tests --- src/Traits/HasPermissions.php | 5 +- src/Traits/HasRoles.php | 7 ++- tests/HasPermissionsTest.php | 23 ++++++++ tests/HasPermissionsWithCustomModelsTest.php | 7 +++ tests/HasRolesTest.php | 62 ++++++++++++++++++++ tests/HasRolesWithCustomModelsTest.php | 22 +++++++ 6 files changed, 123 insertions(+), 3 deletions(-) diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php index fa851e47..a325df50 100644 --- a/src/Traits/HasPermissions.php +++ b/src/Traits/HasPermissions.php @@ -452,7 +452,10 @@ public function syncPermissions(...$permissions) */ public function revokePermissionTo($permission) { - $this->permissions()->detach($this->getStoredPermission($permission)); + $this->permissions()->detach(array_map( + fn ($item) => $this->getStoredPermission($item)->getKey(), + is_a($permission, Collection::class) ? $permission->all() : Arr::wrap($permission) + )); if (is_a($this, Role::class)) { $this->forgetCachedPermissions(); diff --git a/src/Traits/HasRoles.php b/src/Traits/HasRoles.php index 73ab3789..2a1ee54b 100644 --- a/src/Traits/HasRoles.php +++ b/src/Traits/HasRoles.php @@ -180,11 +180,14 @@ function ($object) use ($roles, $model, $teamPivot) { /** * Revoke the given role from the model. * - * @param string|int|Role|\BackedEnum $role + * @param string|string[]|int|Role|Role[]|\BackedEnum $role */ public function removeRole($role) { - $this->roles()->detach($this->getStoredRole($role)); + $this->roles()->detach(array_map( + fn ($item) => $this->getStoredRole($item)->getKey(), + is_a($role, Collection::class) ? $role->all() : Arr::wrap($role) + )); $this->unsetRelation('roles'); diff --git a/tests/HasPermissionsTest.php b/tests/HasPermissionsTest.php index 0eeb961a..ffb6eefe 100644 --- a/tests/HasPermissionsTest.php +++ b/tests/HasPermissionsTest.php @@ -147,6 +147,29 @@ public function it_can_scope_users_using_a_int() $this->assertEquals(2, $scopedUsers3->count()); } + /** @test */ + public function it_can_scope_users_using_a_string_int() + { + User::all()->each(fn ($item) => $item->delete()); + $user1 = User::create(['email' => 'user1@test.com']); + $user2 = User::create(['email' => 'user2@test.com']); + $user3 = User::create(['email' => 'user3@test.com']); + $user1->givePermissionTo(['1', '2']); + $this->testUserRole->givePermissionTo('1'); + $user2->assignRole('testRole'); + + $scopedUsers1 = User::permission('1')->get(); + $scopedUsers2 = User::permission(['2'])->get(); + $scopedUsers3 = User::withoutPermission(['2'])->get(); + + $this->assertEquals(2, $scopedUsers1->count()); + $this->assertEquals(1, $scopedUsers2->count()); + $this->assertEquals(2, $scopedUsers3->count()); + + $user1->revokePermissionTo(['1']); + $this->assertEquals(1, User::permission('1')->count()); + } + /** @test */ public function it_can_scope_users_using_an_array() { diff --git a/tests/HasPermissionsWithCustomModelsTest.php b/tests/HasPermissionsWithCustomModelsTest.php index 60aa481e..583f6d6b 100644 --- a/tests/HasPermissionsWithCustomModelsTest.php +++ b/tests/HasPermissionsWithCustomModelsTest.php @@ -61,6 +61,13 @@ public function it_can_scope_users_using_a_int() $this->assertTrue(true); } + /** @test */ + public function it_can_scope_users_using_a_string_int() + { + // Skipped because custom model uses uuid + $this->assertTrue(true); + } + /** @test */ public function it_can_scope_users_using_a_uuid() { diff --git a/tests/HasRolesTest.php b/tests/HasRolesTest.php index ec7bccc7..94aea8bd 100644 --- a/tests/HasRolesTest.php +++ b/tests/HasRolesTest.php @@ -115,6 +115,42 @@ public function it_can_assign_and_remove_a_role() $this->assertFalse($this->testUser->hasRole('testRole')); } + /** @test */ + public function it_can_assign_and_remove_a_role_using_different_datatypes() + { + $this->assertFalse($this->testUser->hasRole('testRole')); + + $this->testUser->assignRole(1); + $this->assertTrue($this->testUser->hasRole('testRole')); + $this->testUser->removeRole(1); + $this->assertFalse($this->testUser->hasRole('testRole')); + + $this->testUser->assignRole('1'); + $this->assertTrue($this->testUser->hasRole('testRole')); + $this->testUser->removeRole('1'); + $this->assertFalse($this->testUser->hasRole('testRole')); + + $this->testUser->assignRole($this->testUserRole); + $this->assertTrue($this->testUser->hasRole('testRole')); + $this->testUser->removeRole($this->testUserRole); + $this->assertFalse($this->testUser->hasRole('testRole')); + + $this->testUser->assignRole([1]); + $this->assertTrue($this->testUser->hasRole('testRole')); + $this->testUser->removeRole([1]); + $this->assertFalse($this->testUser->hasRole('testRole')); + + $this->testUser->assignRole(['1']); + $this->assertTrue($this->testUser->hasRole('testRole')); + $this->testUser->removeRole(['1']); + $this->assertFalse($this->testUser->hasRole('testRole')); + + $this->testUser->assignRole([$this->testUserRole]); + $this->assertTrue($this->testUser->hasRole('testRole')); + $this->testUser->removeRole([$this->testUserRole]); + $this->assertFalse($this->testUser->hasRole('testRole')); + } + /** @test */ public function it_removes_a_role_and_returns_roles() { @@ -417,6 +453,32 @@ public function it_can_scope_users_using_a_string() $this->assertEquals(1, $scopedUsers->count()); } + /** @test */ + public function it_can_scope_users_using_a_int() + { + $user1 = User::create(['email' => 'user1@test.com']); + $user2 = User::create(['email' => 'user2@test.com']); + $user1->assignRole(1); + $user2->assignRole(2); + + $scopedUsers = User::role(1)->get(); + + $this->assertEquals(1, $scopedUsers->count()); + } + + /** @test */ + public function it_can_scope_users_using_a_string_int() + { + $user1 = User::create(['email' => 'user1@test.com']); + $user2 = User::create(['email' => 'user2@test.com']); + $user1->assignRole('1'); + $user2->assignRole('2'); + + $scopedUsers = User::role('1')->get(); + + $this->assertEquals(1, $scopedUsers->count()); + } + /** @test */ public function it_can_withoutscope_users_using_a_string() { diff --git a/tests/HasRolesWithCustomModelsTest.php b/tests/HasRolesWithCustomModelsTest.php index b306676a..00ef2713 100644 --- a/tests/HasRolesWithCustomModelsTest.php +++ b/tests/HasRolesWithCustomModelsTest.php @@ -30,6 +30,28 @@ public function it_can_use_custom_model_role() $this->assertSame(get_class($this->testUserRole), Role::class); } + /** @test */ + public function it_can_scope_users_using_a_int() + { + // Skipped because custom model uses uuid, + // replacement "it_can_scope_users_using_a_uuid" + $this->assertTrue(true); + } + + /** @test */ + public function it_can_scope_users_using_a_string_int() + { + // Skipped because custom model uses uuid + $this->assertTrue(true); + } + + /** @test */ + public function it_can_assign_and_remove_a_role_using_different_datatypes() + { + // Skipped because custom model uses uuid + $this->assertTrue(true); + } + /** @test */ public function it_doesnt_detach_permissions_when_soft_deleting() { From 6549e4194e3bafe485bab216ae4310cbe1093698 Mon Sep 17 00:00:00 2001 From: erikn69 Date: Mon, 13 Nov 2023 10:37:15 -0500 Subject: [PATCH 3/3] revokePermissionTo, removeRole works with trait collects method --- src/Traits/HasPermissions.php | 5 +---- src/Traits/HasRoles.php | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php index a325df50..b8f8a4c9 100644 --- a/src/Traits/HasPermissions.php +++ b/src/Traits/HasPermissions.php @@ -452,10 +452,7 @@ public function syncPermissions(...$permissions) */ public function revokePermissionTo($permission) { - $this->permissions()->detach(array_map( - fn ($item) => $this->getStoredPermission($item)->getKey(), - is_a($permission, Collection::class) ? $permission->all() : Arr::wrap($permission) - )); + $this->permissions()->detach($this->collectPermissions($permission)); if (is_a($this, Role::class)) { $this->forgetCachedPermissions(); diff --git a/src/Traits/HasRoles.php b/src/Traits/HasRoles.php index 2a1ee54b..f173dab7 100644 --- a/src/Traits/HasRoles.php +++ b/src/Traits/HasRoles.php @@ -184,10 +184,7 @@ function ($object) use ($roles, $model, $teamPivot) { */ public function removeRole($role) { - $this->roles()->detach(array_map( - fn ($item) => $this->getStoredRole($item)->getKey(), - is_a($role, Collection::class) ? $role->all() : Arr::wrap($role) - )); + $this->roles()->detach($this->collectRoles($role)); $this->unsetRelation('roles');