From 80fccb40a65900c44a2c5d38998fb934857a0557 Mon Sep 17 00:00:00 2001 From: Cyrill N Kalita Date: Fri, 17 Oct 2025 22:51:28 +0000 Subject: [PATCH 1/5] Add tests for custom guard sync --- src/Actions/SyncDefinedRole.php | 2 +- tests/Actions/SyncDefinedRoleTest.php | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/Actions/SyncDefinedRole.php b/src/Actions/SyncDefinedRole.php index 2278aee..5fa4b1b 100644 --- a/src/Actions/SyncDefinedRole.php +++ b/src/Actions/SyncDefinedRole.php @@ -21,7 +21,7 @@ public function handle(string $name, string $guard, array $permissions): void $permissions = collect($permissions) ->map(fn ($permission) => match (true) { $permission instanceof BackedEnum => $permission->value, - default => (string) $permission + default => (string) $permission }); $this->role::findOrCreate($name, $guard) diff --git a/tests/Actions/SyncDefinedRoleTest.php b/tests/Actions/SyncDefinedRoleTest.php index 316280e..c643b5f 100644 --- a/tests/Actions/SyncDefinedRoleTest.php +++ b/tests/Actions/SyncDefinedRoleTest.php @@ -28,6 +28,31 @@ public function it_will_defer_syncing_defined_role_to_artisan(): void ]); } + #[Test] + public function it_will_defer_syncing_defined_role_to_artisan_with_custom_guard(): void + { + StorePermission::run('bar', 'admin'); + StorePermission::run(FooAbility::One, 'admin'); + + SyncDefinedRole::run('foo role', 'admin', [ + 'bar', + FooAbility::One, + ]); + + $this->assertDatabaseHas(config('permission.table_names.roles'), [ + 'name' => 'foo role', + 'guard_name' => 'admin', + ]); + + $role = app(config('permission.models.role'))->where([ + 'name' => 'foo role', + 'guard_name' => 'admin', + ])->firstOrFail(); + + $this->assertTrue($role->hasPermissionTo('bar', 'admin')); + $this->assertTrue($role->hasPermissionTo(FooAbility::One, 'admin')); + } + #[Test] public function it_will_throw_an_exception_on_missing_permission(): void { From b88da759722ceb421dea5bffb73c956844a20330 Mon Sep 17 00:00:00 2001 From: Cyrill Kalita Date: Fri, 17 Oct 2025 17:54:50 -0500 Subject: [PATCH 2/5] Apply fixes from StyleCI (#26) Co-authored-by: StyleCI Bot --- src/Actions/SyncDefinedRole.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Actions/SyncDefinedRole.php b/src/Actions/SyncDefinedRole.php index 5fa4b1b..2278aee 100644 --- a/src/Actions/SyncDefinedRole.php +++ b/src/Actions/SyncDefinedRole.php @@ -21,7 +21,7 @@ public function handle(string $name, string $guard, array $permissions): void $permissions = collect($permissions) ->map(fn ($permission) => match (true) { $permission instanceof BackedEnum => $permission->value, - default => (string) $permission + default => (string) $permission }); $this->role::findOrCreate($name, $guard) From efe59bbf9472c7777b94111f34424780ba741a0f Mon Sep 17 00:00:00 2001 From: Cyrill N Kalita Date: Tue, 21 Oct 2025 18:00:58 -0500 Subject: [PATCH 3/5] Adjust the sync logic --- src/Actions/SyncDefinedRole.php | 23 +++++++++++------------ src/Commands/AbilityMakeCommand.php | 7 ++++++- src/Commands/DefinedRoleMakeCommand.php | 2 +- src/Jobs/ResetPermissions.php | 7 ++----- tests/Actions/SyncDefinedRoleTest.php | 14 ++------------ 5 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/Actions/SyncDefinedRole.php b/src/Actions/SyncDefinedRole.php index 2278aee..5a9dddb 100644 --- a/src/Actions/SyncDefinedRole.php +++ b/src/Actions/SyncDefinedRole.php @@ -3,28 +3,27 @@ namespace BinaryCats\LaravelRbac\Actions; use BackedEnum; +use Illuminate\Support\Facades\Artisan; use Lorisleiva\Actions\Action; -use Spatie\Permission\Contracts\Role; +use Spatie\Permission\Commands\CreateRole; class SyncDefinedRole extends Action { - public function __construct( - protected readonly Role $role - ) { - } - /** * Handle syncing a defined role. */ public function handle(string $name, string $guard, array $permissions): void { $permissions = collect($permissions) - ->map(fn ($permission) => match (true) { + ->map(fn ($permission): string => match (true) { $permission instanceof BackedEnum => $permission->value, - default => (string) $permission - }); - - $this->role::findOrCreate($name, $guard) - ->syncPermissions($permissions); + default => (string) $permission + })->implode('|'); + + Artisan::call(CreateRole::class, [ + 'name' => $name, + 'guard' => $guard, + 'permissions' => $permissions + ]); } } diff --git a/src/Commands/AbilityMakeCommand.php b/src/Commands/AbilityMakeCommand.php index 67b99f2..88af981 100644 --- a/src/Commands/AbilityMakeCommand.php +++ b/src/Commands/AbilityMakeCommand.php @@ -40,7 +40,12 @@ protected function getStub() */ protected function getDefaultNamespace($rootNamespace) { - return $rootNamespace.'\Abilities'; + $path = config('rbac.path', 'Abilities'); + + return str($path) + ->after(app()->path()) + ->trim('/') + ->prepend($rootNamespace, '\\'); } /** diff --git a/src/Commands/DefinedRoleMakeCommand.php b/src/Commands/DefinedRoleMakeCommand.php index f6bf224..57ae1ca 100644 --- a/src/Commands/DefinedRoleMakeCommand.php +++ b/src/Commands/DefinedRoleMakeCommand.php @@ -25,7 +25,7 @@ class DefinedRoleMakeCommand extends GeneratorCommand */ protected function getStub() { - return file_exists($customPath = base_path('/stubs/defined-role.stub')) + return file_exists($customPath = $this->laravel->basePath('/stubs/defined-role.stub')) ? $customPath : __DIR__.'/../../stubs/defined-role.stub'; } diff --git a/src/Jobs/ResetPermissions.php b/src/Jobs/ResetPermissions.php index 0d53b52..9471c82 100644 --- a/src/Jobs/ResetPermissions.php +++ b/src/Jobs/ResetPermissions.php @@ -18,18 +18,15 @@ class ResetPermissions use Queueable; use SerializesModels; - protected string $guard; + protected readonly string $guard; - /** - * @param string|null $guard - */ public function __construct(?string $guard = null) { $this->guard = $guard ?? config('auth.defaults.guard'); } /** - * @return void + * Handle resetting permissions */ public function handle(): void { diff --git a/tests/Actions/SyncDefinedRoleTest.php b/tests/Actions/SyncDefinedRoleTest.php index c643b5f..e911cac 100644 --- a/tests/Actions/SyncDefinedRoleTest.php +++ b/tests/Actions/SyncDefinedRoleTest.php @@ -7,7 +7,6 @@ use BinaryCats\LaravelRbac\Tests\Fixtures\Abilities\FooAbility; use BinaryCats\LaravelRbac\Tests\TestCase; use PHPUnit\Framework\Attributes\Test; -use Spatie\Permission\Exceptions\PermissionDoesNotExist; class SyncDefinedRoleTest extends TestCase { @@ -37,6 +36,7 @@ public function it_will_defer_syncing_defined_role_to_artisan_with_custom_guard( SyncDefinedRole::run('foo role', 'admin', [ 'bar', FooAbility::One, + 'this-permission-is-new-and-will-be-created', ]); $this->assertDatabaseHas(config('permission.table_names.roles'), [ @@ -51,16 +51,6 @@ public function it_will_defer_syncing_defined_role_to_artisan_with_custom_guard( $this->assertTrue($role->hasPermissionTo('bar', 'admin')); $this->assertTrue($role->hasPermissionTo(FooAbility::One, 'admin')); - } - - #[Test] - public function it_will_throw_an_exception_on_missing_permission(): void - { - $this->expectException(PermissionDoesNotExist::class); - $this->expectExceptionMessage('There is no permission named `bar` for guard `web`'); - - SyncDefinedRole::run('foo role', 'web', [ - 'bar', - ]); + $this->assertTrue($role->hasPermissionTo('this-permission-is-new-and-will-be-created', 'admin')); } } From 76d8df642ffd51d866ffa032b065ddcdca12761a Mon Sep 17 00:00:00 2001 From: Cyrill Kalita Date: Tue, 21 Oct 2025 18:03:02 -0500 Subject: [PATCH 4/5] Apply fixes from StyleCI (#27) Co-authored-by: StyleCI Bot --- src/Actions/SyncDefinedRole.php | 10 +++++----- src/Jobs/ResetPermissions.php | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Actions/SyncDefinedRole.php b/src/Actions/SyncDefinedRole.php index 5a9dddb..b71e0e8 100644 --- a/src/Actions/SyncDefinedRole.php +++ b/src/Actions/SyncDefinedRole.php @@ -17,13 +17,13 @@ public function handle(string $name, string $guard, array $permissions): void $permissions = collect($permissions) ->map(fn ($permission): string => match (true) { $permission instanceof BackedEnum => $permission->value, - default => (string) $permission + default => (string) $permission })->implode('|'); - + Artisan::call(CreateRole::class, [ - 'name' => $name, - 'guard' => $guard, - 'permissions' => $permissions + 'name' => $name, + 'guard' => $guard, + 'permissions' => $permissions, ]); } } diff --git a/src/Jobs/ResetPermissions.php b/src/Jobs/ResetPermissions.php index 9471c82..80bc83c 100644 --- a/src/Jobs/ResetPermissions.php +++ b/src/Jobs/ResetPermissions.php @@ -26,7 +26,7 @@ public function __construct(?string $guard = null) } /** - * Handle resetting permissions + * Handle resetting permissions. */ public function handle(): void { From 50f83ee3869a2d3f09e0fc210391b37737e6c9c8 Mon Sep 17 00:00:00 2001 From: Cyrill N Kalita Date: Tue, 21 Oct 2025 18:07:50 -0500 Subject: [PATCH 5/5] Adjust the test to include php8.5 --- .github/workflows/run-tests.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index be76279..d5bde82 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -1,7 +1,8 @@ name: run-tests on: - push: + pull_request: + types: [ready_for_review, synchronize] paths: - '**.php' - '.github/workflows/run-tests.yml' @@ -17,7 +18,7 @@ jobs: fail-fast: true matrix: os: [ubuntu-latest] - php: [8.4, 8.3, 8.2] + php: [8.5, 8.4, 8.3, 8.2] laravel: ["^12.0", "^11.0"] stability: [prefer-lowest, prefer-stable] include: