diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php index 7e2b91f53..1b429db81 100644 --- a/src/Traits/HasPermissions.php +++ b/src/Traits/HasPermissions.php @@ -345,7 +345,7 @@ public function getAllPermissions(): Collection * @param string|int|array|\Spatie\Permission\Contracts\Permission|\Illuminate\Support\Collection $permissions * @return array */ - public function collectPermissions(...$permissions) + public function collectPermissions(...$permissions): array { return collect($permissions) ->flatten() @@ -412,9 +412,11 @@ function ($object) use ($permissions, $model) { */ public function syncPermissions(...$permissions) { + $this->collectPermissions(...$permissions); + $this->permissions()->detach(); - return $this->givePermissionTo($permissions); + return $this->givePermissionTo(...$permissions); } /** diff --git a/src/Traits/HasRoles.php b/src/Traits/HasRoles.php index 78787df0f..8c6cb312d 100644 --- a/src/Traits/HasRoles.php +++ b/src/Traits/HasRoles.php @@ -96,14 +96,13 @@ public function scopeRole(Builder $query, $roles, $guard = null): Builder } /** - * Assign the given role to the model. + * Returns roles ids as array keys * - * @param array|string|int|\Spatie\Permission\Contracts\Role|\Illuminate\Support\Collection ...$roles - * @return $this + * @param array|string|int|\Spatie\Permission\Contracts\Role|\Illuminate\Support\Collection $roles */ - public function assignRole(...$roles) + private function collectRoles(...$roles): array { - $roles = collect($roles) + return collect($roles) ->flatten() ->reduce(function ($array, $role) { if (empty($role)) { @@ -122,6 +121,17 @@ public function assignRole(...$roles) return $array; }, []); + } + + /** + * Assign the given role to the model. + * + * @param array|string|int|\Spatie\Permission\Contracts\Role|\Illuminate\Support\Collection ...$roles + * @return $this + */ + public function assignRole(...$roles) + { + $roles = $this->collectRoles(...$roles); $model = $this->getModel(); @@ -175,9 +185,11 @@ public function removeRole($role) */ public function syncRoles(...$roles) { + $this->collectRoles(...$roles); + $this->roles()->detach(); - return $this->assignRole($roles); + return $this->assignRole(...$roles); } /** diff --git a/tests/HasPermissionsTest.php b/tests/HasPermissionsTest.php index 61bba126b..0826b48e6 100644 --- a/tests/HasPermissionsTest.php +++ b/tests/HasPermissionsTest.php @@ -476,6 +476,20 @@ public function sync_permission_ignores_null_inputs() $this->assertFalse($this->testUser->hasDirectPermission('edit-news')); } + /** @test */ + public function sync_permission_does_not_delete_permissions_on_error() + { + $this->testUser->givePermissionTo('edit-news'); + + $this->expectException(PermissionDoesNotExist::class); + + try { + $this->testUser->syncPermissions('edit-news', 'edit-something-that-does-not-exist'); + } finally { + $this->assertTrue($this->testUser->fresh()->hasDirectPermission('edit-news')); + } + } + /** @test */ public function it_does_not_remove_already_associated_permissions_when_assigning_new_permissions() { diff --git a/tests/HasRolesTest.php b/tests/HasRolesTest.php index 0031b58da..8c89a235b 100644 --- a/tests/HasRolesTest.php +++ b/tests/HasRolesTest.php @@ -227,6 +227,20 @@ public function it_will_remove_all_roles_when_an_empty_array_is_passed_to_sync_r $this->assertFalse($this->testUser->hasRole('testRole2')); } + /** @test */ + public function sync_permission_does_not_delete_roles_on_error() + { + $this->testUser->assignRole('testRole'); + + $this->expectException(RoleDoesNotExist::class); + + try { + $this->testUser->syncRoles('testRole2', 'role-that-does-not-exist'); + } finally { + $this->assertTrue($this->testUser->fresh()->hasRole('testRole')); + } + } + /** @test */ public function it_will_sync_roles_to_a_model_that_is_not_persisted() {