From ea6b9c0aad4bcb4bf67190596ce2636f6cf272c8 Mon Sep 17 00:00:00 2001 From: Herpaderp Aldent Date: Thu, 24 Apr 2025 17:54:11 +0200 Subject: [PATCH 1/6] Add RoleType enum casting and refactor type handling logic This commit introduces a cast for the 'type' property in the Role model using the RoleType enum to enforce type safety. It also refactors the BaseRoleService to use the getType() method, improving clarity and reducing reliance on raw value matching. These changes ensure better maintainability and stricter type adherence. --- src/Models/Permissions/Role.php | 5 +++++ src/Services/Roles/BaseRoleService.php | 18 +++++++++++++----- .../Actions/ManageManualRoleActionTest.php | 2 +- tests/Unit/Models/RoleModelTest.php | 2 +- .../Services/Roles/AbstractRoleServiceTest.php | 6 +++--- .../Roles/AutomaticRoleServiceTest.php | 7 ++++--- 6 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/Models/Permissions/Role.php b/src/Models/Permissions/Role.php index 23a0991..f653307 100644 --- a/src/Models/Permissions/Role.php +++ b/src/Models/Permissions/Role.php @@ -27,6 +27,7 @@ namespace Seatplus\Auth\Models\Permissions; use Illuminate\Database\Eloquent\Relations\HasMany; +use Seatplus\Auth\Enums\RoleType; use Seatplus\Auth\Models\AccessControl\RoleMembership; use Spatie\Permission\Models\Role as SpatieRole; @@ -35,6 +36,10 @@ */ class Role extends SpatieRole { + protected $casts = [ + 'type' => RoleType::class + ]; + public function affiliations(): HasMany { return $this->hasMany(Affiliation::class, 'role_id'); diff --git a/src/Services/Roles/BaseRoleService.php b/src/Services/Roles/BaseRoleService.php index 474cefc..86955a8 100644 --- a/src/Services/Roles/BaseRoleService.php +++ b/src/Services/Roles/BaseRoleService.php @@ -57,15 +57,23 @@ public function optIn(): OptInRoleService */ public function getTypeService(): RoleServiceInterface { - return match ($this->role->type) { - RoleType::AUTOMATIC->value => $this->automatic(), - RoleType::ON_REQUEST->value => $this->onRequest(), - RoleType::MANUAL->value => $this->manual(), - RoleType::OPT_IN->value => $this->optIn(), + return match ($this->getType()) { + RoleType::AUTOMATIC => $this->automatic(), + RoleType::ON_REQUEST => $this->onRequest(), + RoleType::MANUAL => $this->manual(), + RoleType::OPT_IN => $this->optIn(), default => throw new \Exception('Role type supported'), }; } + public function getType(): RoleType + { + /* @var RoleType $role_type */ + $role_type = $this->role->type; + + return $role_type; + } + public function handleMembers(): void { $this->getTypeService()->handleMembers(); diff --git a/tests/Unit/Actions/ManageManualRoleActionTest.php b/tests/Unit/Actions/ManageManualRoleActionTest.php index fe10e4c..09bfd47 100644 --- a/tests/Unit/Actions/ManageManualRoleActionTest.php +++ b/tests/Unit/Actions/ManageManualRoleActionTest.php @@ -28,7 +28,7 @@ $action->execute($role_request); - expect($role->refresh()->type)->toBe('manual'); + expect($role->refresh()->type)->toBe(\Seatplus\Auth\Enums\RoleType::MANUAL); }); it('updates the role name', function () { diff --git a/tests/Unit/Models/RoleModelTest.php b/tests/Unit/Models/RoleModelTest.php index 255d7a5..bd764cb 100644 --- a/tests/Unit/Models/RoleModelTest.php +++ b/tests/Unit/Models/RoleModelTest.php @@ -84,7 +84,7 @@ }); it('has default type attribute', function () { - expect(test()->role->fresh()->type)->toEqual('manual'); + expect(test()->role->fresh()->type)->toEqual(\Seatplus\Auth\Enums\RoleType::MANUAL); }); it('has role memberships', function () { diff --git a/tests/Unit/Services/Roles/AbstractRoleServiceTest.php b/tests/Unit/Services/Roles/AbstractRoleServiceTest.php index fb456db..c261d5c 100644 --- a/tests/Unit/Services/Roles/AbstractRoleServiceTest.php +++ b/tests/Unit/Services/Roles/AbstractRoleServiceTest.php @@ -54,7 +54,7 @@ public function canModerate(\Seatplus\Auth\Models\User $user): bool it('returns early when setting same role type', function () { // Arrange - $this->role->type = RoleType::AUTOMATIC->value; + $this->role->type = RoleType::AUTOMATIC; $this->role->save(); // Act @@ -64,7 +64,7 @@ public function canModerate(\Seatplus\Auth\Models\User $user): bool ]); // Assert - expect($this->role->refresh()->type)->toEqual(RoleType::AUTOMATIC->value); + expect($this->role->refresh()->type)->toEqual(RoleType::AUTOMATIC); }); it('sets role type to', function (RoleType $role_type) { @@ -73,7 +73,7 @@ public function canModerate(\Seatplus\Auth\Models\User $user): bool $this->service->setRoleType($role_type); // Assert - expect($this->role->refresh()->type)->toEqual($role_type->value); + expect($this->role->refresh()->type)->toEqual($role_type); })->with([ RoleType::AUTOMATIC, RoleType::ON_REQUEST, diff --git a/tests/Unit/Services/Roles/AutomaticRoleServiceTest.php b/tests/Unit/Services/Roles/AutomaticRoleServiceTest.php index 01121c9..9808686 100644 --- a/tests/Unit/Services/Roles/AutomaticRoleServiceTest.php +++ b/tests/Unit/Services/Roles/AutomaticRoleServiceTest.php @@ -1,5 +1,6 @@ role->type)->toBe('manual'); + expect($this->role->type)->toBe(RoleType::MANUAL); - $this->service->setRoleType(\Seatplus\Auth\Enums\RoleType::AUTOMATIC); + $this->service->setRoleType(RoleType::AUTOMATIC); - expect($this->role->type)->toBe('automatic'); + expect($this->role->type)->toBe(RoleType::AUTOMATIC); }); it('cannot view', function () { From 4574338a6e7367f6065bc1326b847bb7e5936e7b Mon Sep 17 00:00:00 2001 From: Herpaderp Aldent Date: Sun, 27 Apr 2025 10:19:30 +0200 Subject: [PATCH 2/6] remove math arm, that never can be reached --- src/Services/Roles/BaseRoleService.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Services/Roles/BaseRoleService.php b/src/Services/Roles/BaseRoleService.php index 86955a8..cd3dbab 100644 --- a/src/Services/Roles/BaseRoleService.php +++ b/src/Services/Roles/BaseRoleService.php @@ -62,7 +62,6 @@ public function getTypeService(): RoleServiceInterface RoleType::ON_REQUEST => $this->onRequest(), RoleType::MANUAL => $this->manual(), RoleType::OPT_IN => $this->optIn(), - default => throw new \Exception('Role type supported'), }; } From 7c84b6ef48fd4efd3eedba14e8a9a295025cfdf8 Mon Sep 17 00:00:00 2001 From: Herpaderp Aldent Date: Sun, 27 Apr 2025 10:54:45 +0200 Subject: [PATCH 3/6] Update role type comparison and assignment logic Simplify the role type comparison by directly using the roleType object instead of its value property. This change ensures consistency and reduces unnecessary property access. Adjust the update method accordingly to reflect this modification. --- src/Services/Roles/AbstractRoleService.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Services/Roles/AbstractRoleService.php b/src/Services/Roles/AbstractRoleService.php index a058c36..8557644 100644 --- a/src/Services/Roles/AbstractRoleService.php +++ b/src/Services/Roles/AbstractRoleService.php @@ -142,12 +142,12 @@ public function setRoleType(RoleType $roleType): void $originalRoleType = $this->role->type; // if the role type has not changed, we return early - if ($originalRoleType === $roleType->value) { + if ($originalRoleType === $roleType) { return; } $this->role->update([ - 'type' => $roleType->value, + 'type' => $roleType, ]); $this->resetRoleMemberships(); From 42df8a4c45146f69820d94b6397d6dbddef61257 Mon Sep 17 00:00:00 2001 From: Herpaderp Aldent Date: Sun, 27 Apr 2025 10:54:59 +0200 Subject: [PATCH 4/6] Simplify `getType` method in BaseRoleService Replaced the redundant variable assignment in the `getType` method with a direct return statement. This improves code readability and eliminates unnecessary lines without changing functionality. --- src/Services/Roles/BaseRoleService.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Services/Roles/BaseRoleService.php b/src/Services/Roles/BaseRoleService.php index cd3dbab..3e68990 100644 --- a/src/Services/Roles/BaseRoleService.php +++ b/src/Services/Roles/BaseRoleService.php @@ -67,10 +67,7 @@ public function getTypeService(): RoleServiceInterface public function getType(): RoleType { - /* @var RoleType $role_type */ - $role_type = $this->role->type; - - return $role_type; + return $this->role->type; } public function handleMembers(): void From daf4b8239bb13400e18616fee43a05ca0fb39684 Mon Sep 17 00:00:00 2001 From: Herpaderp Aldent Date: Sun, 27 Apr 2025 10:55:24 +0200 Subject: [PATCH 5/6] Fix moderation check in OptInRoleService Replaced the hardcoded `false` with a call to `isModerator`. This ensures the method correctly evaluates whether a user has moderation privileges. --- src/Services/Roles/OptInRoleService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Services/Roles/OptInRoleService.php b/src/Services/Roles/OptInRoleService.php index 74430c2..9e8ec11 100644 --- a/src/Services/Roles/OptInRoleService.php +++ b/src/Services/Roles/OptInRoleService.php @@ -59,6 +59,6 @@ public function canJoin(User $user): bool public function canModerate(User $user): bool { - return false; + return $this->isModerator($user); } } From 0b09f717546d258c2faa92c117144a89f9985f59 Mon Sep 17 00:00:00 2001 From: Herpaderp Aldent Date: Sun, 27 Apr 2025 10:55:34 +0200 Subject: [PATCH 6/6] Adjust Role model property docblock type hint Updated the `Role` model's docblock type hint for `$type` to reflect its usage as `RoleType`. This ensures consistency and clarity in type definitions within the codebase. --- src/Models/Permissions/Role.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Models/Permissions/Role.php b/src/Models/Permissions/Role.php index f653307..752f525 100644 --- a/src/Models/Permissions/Role.php +++ b/src/Models/Permissions/Role.php @@ -32,12 +32,12 @@ use Spatie\Permission\Models\Role as SpatieRole; /** - * @property string $type + * @property RoleType $type */ class Role extends SpatieRole { protected $casts = [ - 'type' => RoleType::class + 'type' => RoleType::class, ]; public function affiliations(): HasMany