Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support strings as numeric ids #2548

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/Traits/HasPermissions.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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()
Expand Down Expand Up @@ -452,7 +452,7 @@ public function syncPermissions(...$permissions)
*/
public function revokePermissionTo($permission)
{
$this->permissions()->detach($this->getStoredPermission($permission));
$this->permissions()->detach($this->collectPermissions($permission));

if (is_a($this, Role::class)) {
$this->forgetCachedPermissions();
Expand Down Expand Up @@ -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());
}

Expand Down
10 changes: 5 additions & 5 deletions src/Traits/HasRoles.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -180,11 +180,11 @@ 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($this->collectRoles($role));

$this->unsetRelation('roles');

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
}

Expand Down
23 changes: 23 additions & 0 deletions tests/HasPermissionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
7 changes: 7 additions & 0 deletions tests/HasPermissionsWithCustomModelsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
62 changes: 62 additions & 0 deletions tests/HasRolesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -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()
{
Expand Down
22 changes: 22 additions & 0 deletions tests/HasRolesWithCustomModelsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down