From 0457922a3092d1fae5b2780232f80d344e002959 Mon Sep 17 00:00:00 2001 From: Wes Copeland Date: Sat, 23 Nov 2024 12:38:12 -0500 Subject: [PATCH 1/5] fix(manage): sync dev role attach/detach with legacy permissions value --- .../Resources/UserResource/Pages/Roles.php | 56 ++++++++++++++++++- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/app/Filament/Resources/UserResource/Pages/Roles.php b/app/Filament/Resources/UserResource/Pages/Roles.php index 61a51156d9..4b7b8df1f1 100644 --- a/app/Filament/Resources/UserResource/Pages/Roles.php +++ b/app/Filament/Resources/UserResource/Pages/Roles.php @@ -4,13 +4,16 @@ namespace App\Filament\Resources\UserResource\Pages; +use App\Enums\Permissions; use App\Filament\Resources\UserResource; use App\Models\Role; +use App\Models\User; use Filament\Resources\Pages\ManageRelatedRecords; use Filament\Tables; use Filament\Tables\Table; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; +use Illuminate\Support\Facades\Auth; use Spatie\Permission\Models\Role as SpatieRole; class Roles extends ManageRelatedRecords @@ -23,6 +26,9 @@ class Roles extends ManageRelatedRecords public function table(Table $table): Table { + /** @var User $user */ + $user = Auth::user(); + // TODO using the resource's table inherits all the actions which open in empty modals // see https://github.com/filamentphp/filament/issues/9492 // return RoleResource::table($table) @@ -37,16 +43,60 @@ public function table(Table $table): Table Tables\Actions\AttachAction::make() ->label(__('Add')) ->color('primary') - ->authorize(fn () => auth()->user()->can('updateRoles', $this->getRecord())) + ->authorize(fn () => $user->can('updateRoles', $this->getRecord())) ->recordTitle(fn (Model $record) => __('permission.role.' . $record->name)) ->preloadRecordSelect() - ->recordSelectOptionsQuery(fn (Builder $query) => $query->whereIn('name', auth()->user()->assignableRoles)), + ->recordSelectOptionsQuery(fn (Builder $query) => $query->whereIn('name', $user->assignableRoles)) + ->after(function ($data) { + /** @var User $targetUser */ + $targetUser = $this->getRecord(); + + $attachedRole = Role::findById((int) $data['recordId']); + + if ($attachedRole->name === Role::DEVELOPER_JUNIOR) { + $targetUser->removeRole(Role::DEVELOPER); + $targetUser->removeRole(Role::DEVELOPER_STAFF); + $targetUser->removeRole(Role::DEVELOPER_VETERAN); + + $targetUser->setAttribute('Permissions', Permissions::JuniorDeveloper); + $targetUser->save(); + } elseif ($attachedRole->name === Role::DEVELOPER) { + $targetUser->removeRole(Role::DEVELOPER_JUNIOR); + $targetUser->removeRole(Role::DEVELOPER_STAFF); + $targetUser->removeRole(Role::DEVELOPER_VETERAN); + + $targetUser->setAttribute('Permissions', Permissions::Developer); + $targetUser->save(); + } elseif ($attachedRole->name === Role::DEVELOPER_STAFF) { + $targetUser->removeRole(Role::DEVELOPER_JUNIOR); + $targetUser->removeRole(Role::DEVELOPER); + $targetUser->removeRole(Role::DEVELOPER_VETERAN); + + $targetUser->setAttribute('Permissions', Permissions::Developer); + $targetUser->save(); + } elseif ($attachedRole->name === Role::DEVELOPER_VETERAN) { + $targetUser->removeRole(Role::DEVELOPER_JUNIOR); + $targetUser->removeRole(Role::DEVELOPER); + $targetUser->removeRole(Role::DEVELOPER_STAFF); + + $targetUser->setAttribute('Permissions', Permissions::Registered); + $targetUser->save(); + } + }), ]) ->paginated(false) ->actions([ Tables\Actions\DetachAction::make() ->label(__('Remove')) - ->authorize(fn (SpatieRole $record) => auth()->user()->can('detachRole', [$this->getRecord(), $record])), + ->authorize(fn (SpatieRole $record) => $user->can('detachRole', [$this->getRecord(), $record])) + ->after(function () { + /** @var User $targetUser */ + $targetUser = $this->getRecord(); + + // Keep legacy permissions in sync. + $targetUser->setAttribute('Permissions', Permissions::Registered); + $targetUser->save(); + }), ]) ->bulkActions([ Tables\Actions\BulkActionGroup::make([]), From 2858093dc25c671cdbcf87a13c5ab03b34a25825 Mon Sep 17 00:00:00 2001 From: Wes Copeland Date: Sun, 24 Nov 2024 12:20:40 -0500 Subject: [PATCH 2/5] fix: address pr feedback --- .../Resources/UserResource/Pages/Roles.php | 24 ++++++++++--------- app/Models/Role.php | 4 ++-- config/roles.php | 4 ++-- lang/en/permission.php | 2 +- .../js/common/utils/generatedAppConstants.ts | 2 +- resources/js/types/generated.d.ts | 4 +++- 6 files changed, 22 insertions(+), 18 deletions(-) diff --git a/app/Filament/Resources/UserResource/Pages/Roles.php b/app/Filament/Resources/UserResource/Pages/Roles.php index 4b7b8df1f1..d39c83e9c3 100644 --- a/app/Filament/Resources/UserResource/Pages/Roles.php +++ b/app/Filament/Resources/UserResource/Pages/Roles.php @@ -53,33 +53,35 @@ public function table(Table $table): Table $attachedRole = Role::findById((int) $data['recordId']); + $newPermissions = Permissions::Registered; if ($attachedRole->name === Role::DEVELOPER_JUNIOR) { $targetUser->removeRole(Role::DEVELOPER); $targetUser->removeRole(Role::DEVELOPER_STAFF); - $targetUser->removeRole(Role::DEVELOPER_VETERAN); + $targetUser->removeRole(Role::DEVELOPER_RETIRED); - $targetUser->setAttribute('Permissions', Permissions::JuniorDeveloper); - $targetUser->save(); + $newPermissions = Permissions::JuniorDeveloper; } elseif ($attachedRole->name === Role::DEVELOPER) { $targetUser->removeRole(Role::DEVELOPER_JUNIOR); $targetUser->removeRole(Role::DEVELOPER_STAFF); - $targetUser->removeRole(Role::DEVELOPER_VETERAN); + $targetUser->removeRole(Role::DEVELOPER_RETIRED); - $targetUser->setAttribute('Permissions', Permissions::Developer); - $targetUser->save(); + $newPermissions = Permissions::Developer; } elseif ($attachedRole->name === Role::DEVELOPER_STAFF) { $targetUser->removeRole(Role::DEVELOPER_JUNIOR); $targetUser->removeRole(Role::DEVELOPER); - $targetUser->removeRole(Role::DEVELOPER_VETERAN); + $targetUser->removeRole(Role::DEVELOPER_RETIRED); - $targetUser->setAttribute('Permissions', Permissions::Developer); - $targetUser->save(); - } elseif ($attachedRole->name === Role::DEVELOPER_VETERAN) { + $newPermissions = Permissions::Developer; + } elseif ($attachedRole->name === Role::DEVELOPER_RETIRED) { $targetUser->removeRole(Role::DEVELOPER_JUNIOR); $targetUser->removeRole(Role::DEVELOPER); $targetUser->removeRole(Role::DEVELOPER_STAFF); + } - $targetUser->setAttribute('Permissions', Permissions::Registered); + $currentPermissions = (int) $targetUser->getAttribute('Permissions'); + // Don't strip moderation power away if the user already has it. + if ($currentPermissions < Permissions::Moderator) { + $targetUser->setAttribute('Permissions', $newPermissions); $targetUser->save(); } }), diff --git a/app/Models/Role.php b/app/Models/Role.php index b562188d5e..78ec44c565 100644 --- a/app/Models/Role.php +++ b/app/Models/Role.php @@ -75,7 +75,7 @@ class Role extends \Spatie\Permission\Models\Role // vanity roles assigned by admin - public const DEVELOPER_VETERAN = 'developer-veteran'; + public const DEVELOPER_RETIRED = 'developer-retired'; public static function toFilamentColor(string $role): string { @@ -117,7 +117,7 @@ public static function toFilamentColor(string $role): string // vanity roles assigned by admin - Role::DEVELOPER_VETERAN => 'primary', + Role::DEVELOPER_RETIRED => 'primary', default => 'gray', }; } diff --git a/config/roles.php b/config/roles.php index 5ad6e33b87..df30347a01 100644 --- a/config/roles.php +++ b/config/roles.php @@ -18,7 +18,7 @@ Role::CHEAT_INVESTIGATOR, Role::DEVELOPER_JUNIOR, Role::DEVELOPER_STAFF, - Role::DEVELOPER_VETERAN, + Role::DEVELOPER_RETIRED, Role::DEVELOPER, Role::EVENT_MANAGER, Role::FORUM_MANAGER, @@ -183,7 +183,7 @@ // vanity roles assigned by admin [ - 'name' => Role::DEVELOPER_VETERAN, + 'name' => Role::DEVELOPER_RETIRED, 'display' => 5, 'legacy_role' => Permissions::Registered, ], diff --git a/lang/en/permission.php b/lang/en/permission.php index 6fd6b7cee1..6d5d44d128 100755 --- a/lang/en/permission.php +++ b/lang/en/permission.php @@ -41,6 +41,6 @@ // vanity roles assigned by admins - Role::DEVELOPER_VETERAN => __('Veteran Developer'), + Role::DEVELOPER_RETIRED => __('Developer (retired)'), ], ]; diff --git a/resources/js/common/utils/generatedAppConstants.ts b/resources/js/common/utils/generatedAppConstants.ts index 22e5427d79..352c6a2e22 100644 --- a/resources/js/common/utils/generatedAppConstants.ts +++ b/resources/js/common/utils/generatedAppConstants.ts @@ -81,7 +81,7 @@ export const UserRole = { ENGINEER: 'engineer', TEAM_ACCOUNT: 'team-account', BETA: 'beta', - DEVELOPER_VETERAN: 'developer-veteran', + DEVELOPER_RETIRED: 'developer-retired', } as const; diff --git a/resources/js/types/generated.d.ts b/resources/js/types/generated.d.ts index 6b14c233ed..bf47c63709 100644 --- a/resources/js/types/generated.d.ts +++ b/resources/js/types/generated.d.ts @@ -182,6 +182,7 @@ declare namespace App.Data { }; } declare namespace App.Enums { + export type ClientSupportLevel = 0 | 1 | 2 | 3; export type UserPreference = | 0 | 1 @@ -240,7 +241,7 @@ declare namespace App.Models { | 'engineer' | 'team-account' | 'beta' - | 'developer-veteran'; + | 'developer-retired'; } declare namespace App.Platform.Data { export type Achievement = { @@ -370,6 +371,7 @@ declare namespace App.Platform.Data { }; } declare namespace App.Platform.Enums { + export type AchievementAuthorTask = 'artwork' | 'design' | 'logic' | 'testing' | 'writing'; export type UnlockMode = 0 | 1; export type AchievementFlag = 3 | 5; export type AchievementSetType = From 28fc3ce9371fd251a4326de5c7e852b8f618d887 Mon Sep 17 00:00:00 2001 From: Wes Copeland Date: Fri, 29 Nov 2024 16:26:08 -0500 Subject: [PATCH 3/5] fix: address pr feedback --- .../Resources/UserResource/Pages/Roles.php | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/app/Filament/Resources/UserResource/Pages/Roles.php b/app/Filament/Resources/UserResource/Pages/Roles.php index d39c83e9c3..7475b6f542 100644 --- a/app/Filament/Resources/UserResource/Pages/Roles.php +++ b/app/Filament/Resources/UserResource/Pages/Roles.php @@ -53,6 +53,15 @@ public function table(Table $table): Table $attachedRole = Role::findById((int) $data['recordId']); + if (!in_array($attachedRole->name, [ + Role::DEVELOPER_JUNIOR, + Role::DEVELOPER, + Role::DEVELOPER_STAFF, + Role::DEVELOPER_RETIRED, + ])) { + return; + } + $newPermissions = Permissions::Registered; if ($attachedRole->name === Role::DEVELOPER_JUNIOR) { $targetUser->removeRole(Role::DEVELOPER); @@ -91,13 +100,28 @@ public function table(Table $table): Table Tables\Actions\DetachAction::make() ->label(__('Remove')) ->authorize(fn (SpatieRole $record) => $user->can('detachRole', [$this->getRecord(), $record])) - ->after(function () { + ->after(function (SpatieRole $record) { /** @var User $targetUser */ $targetUser = $this->getRecord(); + // Only reset permissions if it's a developer role being removed. + // Users can only have a single kind of developer role attached. + if (!in_array($record->name, [ + Role::DEVELOPER_JUNIOR, + Role::DEVELOPER, + Role::DEVELOPER_STAFF, + Role::DEVELOPER_RETIRED, + ])) { + return; + } + // Keep legacy permissions in sync. - $targetUser->setAttribute('Permissions', Permissions::Registered); - $targetUser->save(); + $currentPermissions = (int) $targetUser->getAttribute('Permissions'); + // Don't strip moderation power away if the user already has it. + if ($currentPermissions < Permissions::Moderator) { + $targetUser->setAttribute('Permissions', Permissions::Registered); + $targetUser->save(); + } }), ]) ->bulkActions([ From 49ab2a652a9c48a483dc2740ad8f07ae70c94809 Mon Sep 17 00:00:00 2001 From: Wes Copeland Date: Sat, 30 Nov 2024 16:29:40 -0500 Subject: [PATCH 4/5] fix: address pr feedback --- .../Resources/UserResource/Pages/Roles.php | 66 +++++++++---------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/app/Filament/Resources/UserResource/Pages/Roles.php b/app/Filament/Resources/UserResource/Pages/Roles.php index 7475b6f542..3ea7200ee1 100644 --- a/app/Filament/Resources/UserResource/Pages/Roles.php +++ b/app/Filament/Resources/UserResource/Pages/Roles.php @@ -53,45 +53,43 @@ public function table(Table $table): Table $attachedRole = Role::findById((int) $data['recordId']); - if (!in_array($attachedRole->name, [ + if (in_array($attachedRole->name, [ Role::DEVELOPER_JUNIOR, Role::DEVELOPER, Role::DEVELOPER_STAFF, Role::DEVELOPER_RETIRED, ])) { - return; - } - - $newPermissions = Permissions::Registered; - if ($attachedRole->name === Role::DEVELOPER_JUNIOR) { - $targetUser->removeRole(Role::DEVELOPER); - $targetUser->removeRole(Role::DEVELOPER_STAFF); - $targetUser->removeRole(Role::DEVELOPER_RETIRED); - - $newPermissions = Permissions::JuniorDeveloper; - } elseif ($attachedRole->name === Role::DEVELOPER) { - $targetUser->removeRole(Role::DEVELOPER_JUNIOR); - $targetUser->removeRole(Role::DEVELOPER_STAFF); - $targetUser->removeRole(Role::DEVELOPER_RETIRED); - - $newPermissions = Permissions::Developer; - } elseif ($attachedRole->name === Role::DEVELOPER_STAFF) { - $targetUser->removeRole(Role::DEVELOPER_JUNIOR); - $targetUser->removeRole(Role::DEVELOPER); - $targetUser->removeRole(Role::DEVELOPER_RETIRED); - - $newPermissions = Permissions::Developer; - } elseif ($attachedRole->name === Role::DEVELOPER_RETIRED) { - $targetUser->removeRole(Role::DEVELOPER_JUNIOR); - $targetUser->removeRole(Role::DEVELOPER); - $targetUser->removeRole(Role::DEVELOPER_STAFF); - } - - $currentPermissions = (int) $targetUser->getAttribute('Permissions'); - // Don't strip moderation power away if the user already has it. - if ($currentPermissions < Permissions::Moderator) { - $targetUser->setAttribute('Permissions', $newPermissions); - $targetUser->save(); + $newPermissions = Permissions::Registered; + if ($attachedRole->name === Role::DEVELOPER_JUNIOR) { + $targetUser->removeRole(Role::DEVELOPER); + $targetUser->removeRole(Role::DEVELOPER_STAFF); + $targetUser->removeRole(Role::DEVELOPER_RETIRED); + + $newPermissions = Permissions::JuniorDeveloper; + } elseif ($attachedRole->name === Role::DEVELOPER) { + $targetUser->removeRole(Role::DEVELOPER_JUNIOR); + $targetUser->removeRole(Role::DEVELOPER_STAFF); + $targetUser->removeRole(Role::DEVELOPER_RETIRED); + + $newPermissions = Permissions::Developer; + } elseif ($attachedRole->name === Role::DEVELOPER_STAFF) { + $targetUser->removeRole(Role::DEVELOPER_JUNIOR); + $targetUser->removeRole(Role::DEVELOPER); + $targetUser->removeRole(Role::DEVELOPER_RETIRED); + + $newPermissions = Permissions::Developer; + } elseif ($attachedRole->name === Role::DEVELOPER_RETIRED) { + $targetUser->removeRole(Role::DEVELOPER_JUNIOR); + $targetUser->removeRole(Role::DEVELOPER); + $targetUser->removeRole(Role::DEVELOPER_STAFF); + } + + $currentPermissions = (int) $targetUser->getAttribute('Permissions'); + // Don't strip moderation power away if the user already has it. + if ($currentPermissions < Permissions::Moderator) { + $targetUser->setAttribute('Permissions', $newPermissions); + $targetUser->save(); + } } }), ]) From da22fd8ce85b3a44b82f20684e231bb94f3098c8 Mon Sep 17 00:00:00 2001 From: Wes Copeland Date: Sat, 30 Nov 2024 16:31:51 -0500 Subject: [PATCH 5/5] fix: add an explicit else return --- app/Filament/Resources/UserResource/Pages/Roles.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/Filament/Resources/UserResource/Pages/Roles.php b/app/Filament/Resources/UserResource/Pages/Roles.php index 3ea7200ee1..653590dd5b 100644 --- a/app/Filament/Resources/UserResource/Pages/Roles.php +++ b/app/Filament/Resources/UserResource/Pages/Roles.php @@ -90,6 +90,9 @@ public function table(Table $table): Table $targetUser->setAttribute('Permissions', $newPermissions); $targetUser->save(); } + } else { + // There's nothing to synchronize for non-developer roles. + return; } }), ])