Skip to content

Commit 1223e3d

Browse files
author
alexphila
committed
renamed ambiguous functions, modified tests to use custom model, fixed scope to use identifier when configured
1 parent 2a1f631 commit 1223e3d

File tree

6 files changed

+71
-65
lines changed

6 files changed

+71
-65
lines changed

‎database/migrations/create_permission_tables.php.stub

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ return new class extends Migration
3838
$table->index($columnNames['team_foreign_key'], 'roles_team_foreign_key_index');
3939
}
4040
$table->string('name'); // For MyISAM use string('name', 225); // (or 166 for InnoDB with Redundant/Compact row format)
41-
$table->string('slug')->nullable();
4241
$table->string('guard_name'); // For MyISAM use string('guard_name', 25);
4342
$table->timestamps();
4443
if ($teams || config('permission.testing')) {

‎src/Models/Role.php

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,18 +127,18 @@ public static function findById(int|string $id, ?string $guardName = null): Role
127127
}
128128

129129
/**
130-
* Find a role by its configured key (e.g. name or slug) and guard name.
130+
* Find a role by its configured identifier (e.g. name or slug) and guard name.
131131
*
132132
* @return RoleContract|Role
133133
*
134134
* @throws RoleDoesNotExist
135135
*/
136-
public static function findByKey(string $value, ?string $guardName = null): RoleContract
136+
public static function findByIdentifier(string $value, ?string $guardName = null): RoleContract
137137
{
138138
$guardName = $guardName ?? Guard::getDefaultName(static::class);
139-
$key = config('permission.role_identifier', 'name');
139+
$identifier = config('permission.role_identifier', 'name');
140140

141-
$role = static::findByParam([$key => $value, 'guard_name' => $guardName]);
141+
$role = static::findByParam([$identifier => $value, 'guard_name' => $guardName]);
142142

143143
if (! $role) {
144144
throw RoleDoesNotExist::named($value, $guardName);
@@ -153,14 +153,16 @@ public static function findByKey(string $value, ?string $guardName = null): Role
153153
*
154154
* @return RoleContract|Role
155155
*/
156-
public static function findOrCreate(string $name, ?string $guardName = null): RoleContract
156+
public static function findOrCreate(string $value, ?string $guardName = null): RoleContract
157157
{
158158
$guardName = $guardName ?? Guard::getDefaultName(static::class);
159159

160-
$role = static::findByParam(['name' => $name, 'guard_name' => $guardName]);
160+
$identifier = config('permission.role_identifier', 'name');
161+
162+
$role = static::findByParam([$identifier => $value, 'guard_name' => $guardName]);
161163

162164
if (! $role) {
163-
return static::query()->create(['name' => $name, 'guard_name' => $guardName] + (app(PermissionRegistrar::class)->teams ? [app(PermissionRegistrar::class)->teamsKey => getPermissionsTeamId()] : []));
165+
return static::query()->create([$identifier => $value, 'guard_name' => $guardName] + (app(PermissionRegistrar::class)->teams ? [app(PermissionRegistrar::class)->teamsKey => getPermissionsTeamId()] : []));
164166
}
165167

166168
return $role;

‎src/Traits/HasRoles.php

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,17 @@ public function scopeRole(Builder $query, $roles, $guard = null, $without = fals
9191
$role = $role->value;
9292
}
9393

94-
$method = is_int($role) || PermissionRegistrar::isUid($role) ? 'findById' : 'findByName';
94+
$roleIdentifier = $this->getRoleIdentifier();
95+
96+
$method = '';
97+
98+
if (is_int($role) || PermissionRegistrar::isUid($role)) {
99+
$method = 'findById';
100+
} else if ($roleIdentifier !== 'name') {
101+
$method = 'findByIdentifier';
102+
} else {
103+
$method = 'findByName';
104+
}
95105

96106
return $this->getRoleClass()::{$method}($role, $guard ?: $this->getDefaultGuardName());
97107
}, Arr::wrap($roles));
@@ -238,7 +248,7 @@ public function syncRoles(...$roles)
238248
*/
239249
public function hasRole($roles, ?string $guard = null): bool
240250
{
241-
$roleKey = $this->getRoleKey();
251+
$roleKey = $this->getRoleIdentifier();
242252

243253
$this->loadMissing('roles');
244254

@@ -316,7 +326,7 @@ public function hasAnyRole(...$roles): bool
316326
*/
317327
public function hasAllRoles($roles, ?string $guard = null): bool
318328
{
319-
$roleKey = $this->getRoleKey();
329+
$roleKey = $this->getRoleIdentifier();
320330

321331
$this->loadMissing('roles');
322332

@@ -366,7 +376,7 @@ public function hasAllRoles($roles, ?string $guard = null): bool
366376
*/
367377
public function hasExactRoles($roles, ?string $guard = null): bool
368378
{
369-
$roleKey = $this->getRoleKey();
379+
$roleKey = $this->getRoleIdentifier();
370380

371381
$this->loadMissing('roles');
372382

@@ -406,7 +416,7 @@ public function getRoleNames(): Collection
406416
{
407417
$this->loadMissing('roles');
408418

409-
return $this->roles->pluck($this->getRoleKey());
419+
return $this->roles->pluck($this->getRoleIdentifier());
410420
}
411421

412422
protected function getStoredRole($role): Role
@@ -420,7 +430,7 @@ protected function getStoredRole($role): Role
420430
}
421431

422432
if (is_string($role)) {
423-
return $this->getRoleClass()::findByKey($role, $this->getDefaultGuardName());
433+
return $this->getRoleClass()::findByIdentifier($role, $this->getDefaultGuardName());
424434
}
425435

426436
return $role;
@@ -448,7 +458,7 @@ protected function convertPipeToArray(string $pipeString)
448458
return explode('|', trim($pipeString, $quoteCharacter));
449459
}
450460

451-
protected function getRoleKey(): string
461+
protected function getRoleIdentifier(): string
452462
{
453463
return config('permission.role_identifier', 'name');
454464
}

‎tests/HasRolesTest.php

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -46,56 +46,6 @@ public function it_can_determine_that_the_user_does_not_have_a_role()
4646
$role = app(Role::class)->findOrCreate('testRoleInWebGuard2', 'web');
4747
$this->assertFalse($this->testUser->hasRole($role));
4848
}
49-
50-
/** @test */
51-
#[Test]
52-
public function test_has_role_with_default_identifier()
53-
{
54-
config(['permission.role_identifier' => 'name']);
55-
56-
$user = User::create(['email' => '[email protected]']);
57-
58-
app(Role::class)->create(['name' => 'Editor', 'slug' => 'editor', 'guard_name' => 'web']);
59-
60-
$user->assignRole('Editor');
61-
62-
$this->assertTrue($user->hasRole('Editor'));
63-
$this->assertFalse($user->hasRole('editor')); // slug shouldn't work
64-
}
65-
66-
/** @test */
67-
#[Test]
68-
public function test_has_role_with_slug_identifier()
69-
{
70-
config(['permission.role_identifier' => 'slug']);
71-
72-
$user = User::create(['email' => '[email protected]']);
73-
74-
app(Role::class)->create(['name' => 'Editor', 'slug' => 'editor', 'guard_name' => 'web']);
75-
76-
$user->assignRole('editor');
77-
78-
$this->assertTrue($user->hasRole('editor'));
79-
$this->assertFalse($user->hasRole('Editor')); // name shouldn't work
80-
}
81-
82-
/** @test */
83-
#[Test]
84-
public function test_has_all_roles_with_slug_identifier()
85-
{
86-
config(['permission.role_identifier' => 'slug']);
87-
88-
$user = User::create(['email' => '[email protected]']);
89-
90-
app(Role::class)->create(['name' => 'Admin', 'slug' => 'admin', 'guard_name' => 'web']);
91-
app(Role::class)->create(['name' => 'Manager', 'slug' => 'manager', 'guard_name' => 'web']);
92-
93-
$user->assignRole('admin', 'manager');
94-
95-
$this->assertTrue($user->hasAllRoles(['admin', 'manager']));
96-
$this->assertTrue($user->hasExactRoles(['admin', 'manager']));
97-
$this->assertFalse($user->hasExactRoles(['admin']));
98-
}
9949

10050
/**
10151
* @test

‎tests/HasRolesWithCustomModelsTest.php

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Illuminate\Support\Carbon;
66
use Illuminate\Support\Facades\DB;
77
use PHPUnit\Framework\Attributes\Test;
8+
use Spatie\Permission\Exceptions\RoleDoesNotExist;
89
use Spatie\Permission\Tests\TestModels\Admin;
910
use Spatie\Permission\Tests\TestModels\Role;
1011

@@ -106,4 +107,47 @@ public function it_should_touch_when_assigning_new_roles()
106107
$this->assertSame('2021-07-20 19:13:14', $role1->refresh()->updated_at->format('Y-m-d H:i:s'));
107108
$this->assertSame('2021-07-20 19:13:14', $role2->refresh()->updated_at->format('Y-m-d H:i:s'));
108109
}
110+
111+
/** @test */
112+
#[Test]
113+
public function it_can_assign_and_remove_a_role_with_custom_identifier()
114+
{
115+
config(['permission.role_identifier' => 'slug']);
116+
117+
app(Role::class)->create(['name' => 'Editor', 'slug' => 'editor', 'guard_name' => 'web']);
118+
app(Role::class)->create(['name' => 'Writer', 'slug' => 'writer', 'guard_name' => 'web']);
119+
120+
$this->assertFalse($this->testUser->hasRole('editor'));
121+
$this->assertFalse($this->testUser->hasRole('Editor'));
122+
123+
$this->testUser->assignRole('editor');
124+
$this->testUser->assignRole('writer');
125+
126+
$this->assertTrue($this->testUser->hasRole('editor'));
127+
$this->assertFalse($this->testUser->hasRole('Editor'));
128+
129+
$this->assertTrue($this->testUser->hasAllRoles(['editor']));
130+
$this->assertTrue($this->testUser->hasAllRoles(['editor', 'writer']));
131+
$this->assertFalse($this->testUser->hasAllRoles(['editor', 'writer', 'not exist']));
132+
133+
$this->assertTrue($this->testUser->hasExactRoles(['editor', 'writer']));
134+
$this->assertFalse($this->testUser->hasExactRoles(['editor']));
135+
136+
$this->testUser->removeRole('editor');
137+
138+
$this->assertFalse($this->testUser->hasRole('editor'));
139+
}
140+
141+
/** @test */
142+
#[Test]
143+
public function it_throws_an_exception_when_assigning_a_role_by_name_when_identifier_is_not_name()
144+
{
145+
config(['permission.role_identifier' => 'slug']);
146+
147+
app(Role::class)->create(['name' => 'Editor', 'slug' => 'editor', 'guard_name' => 'web']);
148+
149+
$this->expectException(RoleDoesNotExist::class);
150+
151+
$this->testUser->assignRole('Editor');
152+
}
109153
}

‎tests/TestCase.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ protected function setUpDatabase($app)
190190

191191
$schema->table(config('permission.table_names.roles'), function (Blueprint $table) {
192192
$table->softDeletes();
193+
$table->string("slug")->unique()->nullable();
193194
});
194195
$schema->table(config('permission.table_names.permissions'), function (Blueprint $table) {
195196
$table->softDeletes();

0 commit comments

Comments
 (0)