feat(panel-admin): UserResource com List/View/Edit e Role hierarchy#455
Open
YuriSouzaDev wants to merge 1 commit into
Open
feat(panel-admin): UserResource com List/View/Edit e Role hierarchy#455YuriSouzaDev wants to merge 1 commit into
YuriSouzaDev wants to merge 1 commit into
Conversation
Adiciona uma tela única no /admin para staff visualizar e editar um
membro por inteiro, agregando dados hoje espalhados entre Character,
ExternalIdentity, Profile, Address e ModerationCase — mantendo a
fronteira presentation/core (seções agregadas são lidas via
relacionamento, sem duplicar lógica de escrita de outros domínios).
Identity:
- Enum Role (Staff, Compliance, Recruiter, SquadCaptain, Member) com
hierarquia isStaff()/isCompliance()/canViewUsers().
- SoftDeletes no User + migration adicionando `role` e `deleted_at`;
unique index de `username` vira parcial (WHERE deleted_at IS NULL)
para não travar reuso de username por conta soft-deletada.
- UserPolicy: viewAny/view liberam staff/compliance/recruiter/squad
captain; update/delete restritos a staff; restore/forceDelete
restritos a compliance (hard delete nunca é o padrão).
- Relações profile()/workExperiences()/profileSkills() no User.
Panel-admin (UserResource, sem Create — contas só nascem via OAuth):
- List: colunas de senioridade/disponibilidade/cidade/nível/status
computado (ativo/suspenso/banido/removido), paginação [25,50,100],
filtros de role/senioridade/disponibilidade/trashed.
- Edit: identidade (username/name/email/role/is_donator), perfil
profissional via Section::relationship('profile') com hooks pra
achatar/reagrupar o cast custom de preferences, e endereço via
Section::relationship('address').
- View: mesmos dados em modo leitura, mais Gamificação/Atividade/
Moderação agregadas por relacionamento; seção de Moderação oculta
para quem não é staff.
- RelationManagers de Skills e Experiências (create/edit/delete
staff-only; somente leitura para recruiter/squad captain).
Testes: UserPolicyTest cobrindo a hierarquia de roles; UserResource-
Test cobrindo autorização por página/seção, edição multi-seção com
persistência de preferences/social_links/endereço, relation managers,
soft delete padrão e hard delete restrito a compliance.
📝 WalkthroughWalkthroughAdds role-based user authorization, soft deletes, localized role labels, and profile relationships. Introduces a Filament Users resource with list, view, edit, forms, tables, infolists, relation managers, and admin navigation registration. Adds feature and unit tests covering permissions, profile data, related records, moderation visibility, and deletion behavior. Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
app-modules/panel-admin/src/Filament/Resources/Users/UserResource.php (1)
77-80: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueGlobal search over
Recruiters/squad captains get read access to this resource; global search results render matched attributes. Consider limiting to
username/name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/panel-admin/src/Filament/Resources/Users/UserResource.php` around lines 77 - 80, Update UserResource::getGloballySearchableAttributes() to remove email from the globally searchable attributes, retaining only username and name so global search does not expose email values to panel roles.app-modules/panel-admin/src/Filament/Resources/Users/Tables/UsersTable.php (1)
113-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
operator:named argument is being used to pass a value.It works only because Eloquent re-shuffles a non-string operator into the value slot. Pass it positionally as the value.
Proposed fix
- true: static fn (Builder $query): Builder => $query->whereRelation('profile', 'available_for_proposals', operator: true), - false: static fn (Builder $query): Builder => $query->whereRelation('profile', 'available_for_proposals', operator: false), + true: static fn (Builder $query): Builder => $query->whereRelation('profile', 'available_for_proposals', '=', true), + false: static fn (Builder $query): Builder => $query->whereRelation('profile', 'available_for_proposals', '=', false),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/panel-admin/src/Filament/Resources/Users/Tables/UsersTable.php` around lines 113 - 116, Update the true and false callbacks in the queries configuration to pass the boolean available_for_proposals values positionally to whereRelation instead of using the operator named argument; preserve the existing profile relationship and filtering behavior.app-modules/panel-admin/tests/Feature/Users/UserResourceTest.php (3)
27-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLoop hides which role failed.
Use a Pest dataset so failures name the role.
Proposed fix
-test('staff, recruiter e squad captain listam os usuários', function (): void { +test('staff, recruiter e squad captain listam os usuários', function (string $state): void { $member = User::factory()->create(); - foreach (['staff', 'recruiter', 'squadCaptain'] as $state) { - $viewer = User::factory()->{$state}()->create(); + $viewer = User::factory()->{$state}()->create(); - $this->actingAs($viewer); + $this->actingAs($viewer); - livewire(ListUsers::class) - ->loadTable() - ->assertCanSeeTableRecords([$viewer, $member]); - } -}); + livewire(ListUsers::class) + ->loadTable() + ->assertCanSeeTableRecords([$viewer, $member]); +})->with(['staff', 'recruiter', 'squadCaptain']);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/panel-admin/tests/Feature/Users/UserResourceTest.php` around lines 27 - 39, Replace the foreach in the “staff, recruiter e squad captain listam os usuários” test with a Pest dataset that supplies each role to a separate test case, preserving the existing viewer creation, authentication, and table assertions so failures identify the specific role.
90-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for the
nameuniqueness rule flagged inUserForm.php.No test exercises editing a user whose
namecollides with another — add one, or drop the rule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/panel-admin/tests/Feature/Users/UserResourceTest.php` around lines 90 - 128, Extend the user edit feature tests around EditUser to cover the UserForm name uniqueness rule: create two users, edit one while assigning the other’s name, submit the form, and assert the validation error is present and the update is not accepted. Keep the existing successful multi-field submission test unchanged.
279-298: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest asserts less than its name claims.
Only
createis checked;edit/deletevisibility is untested. Seed aWorkExperience/ProfileSkilland assert those actions are hidden too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/panel-admin/tests/Feature/Users/UserResourceTest.php` around lines 279 - 298, Expand the test “recrutador não vê ações de criar/editar/excluir nas relation managers” by creating representative WorkExperience and ProfileSkill records for the target, then assert the create, edit, and delete table actions are hidden in both WorkExperiencesRelationManager and ProfileSkillsRelationManager. Keep the existing authentication and page setup unchanged.app-modules/panel-admin/src/Filament/Resources/Users/Schemas/UserInfolist.php (2)
49-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated inline staff check.
auth()->user()?->role->isStaff() ?? falseis repeated here and in both relation managers. Extract a single helper/gate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/panel-admin/src/Filament/Resources/Users/Schemas/UserInfolist.php` at line 49, Extract the repeated `auth()->user()?->role->isStaff() ?? false` check into a shared helper or gate, then update the visibility callback in the UserInfolist and both relation managers to reuse it. Preserve the existing boolean behavior for authenticated staff and all other users.
366-369: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMagic string
'joined'for voice state.Use the state enum instead of a literal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/panel-admin/src/Filament/Resources/Users/Schemas/UserInfolist.php` around lines 366 - 369, Update the Voice query in UserInfolist to replace the literal 'joined' state with the corresponding Voice state enum value, reusing the existing enum symbol and leaving the query behavior unchanged.app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/WorkExperiencesRelationManager.php (1)
103-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStaff authorization is inlined in three places instead of going through
UserPolicy.auth()->user()?->role->isStaff() ?? falseis copy-pasted, so a role-hierarchy change must be applied in every copy and never gets policy coverage.
app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/WorkExperiencesRelationManager.php#L103-L106: replace the method body with a Gate/policy ability check (e.g.Gate::allows('update', $this->getOwnerRecord())).app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/ProfileSkillsRelationManager.php#L84-L87: same replacement.app-modules/panel-admin/src/Filament/Resources/Users/Schemas/UserInfolist.php#L49-L49: gate the Moderação tab on the same shared ability instead of the inline role check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/WorkExperiencesRelationManager.php` around lines 103 - 106, Replace the inline staff-role checks with the shared UserPolicy ability check: update isEditableByCurrentUser() in app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/WorkExperiencesRelationManager.php:103-106 and ProfileSkillsRelationManager.php:84-87 to use Gate::allows('update', $this->getOwnerRecord()), and update the Moderação tab condition in UserInfolist.php:49 to use the same ability. Ensure the required Gate import is present in each affected file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app-modules/identity/database/migrations/2026_07_26_120000_add_role_and_soft_deletes_to_users_table.php`:
- Around line 32-34: Update the rollback logic in the users-table migration
before applying unique('username') to reconcile soft-deleted duplicate
usernames, such as by renaming or otherwise separating deleted duplicates; then
restore the global username constraint and remove the role and deleted_at
columns.
In `@app-modules/identity/src/User/Models/User.php`:
- Line 61: Update the User model’s deletion behavior around SoftDeletes and
UserObserver::deleted() so address deletion occurs only when the user is
force-deleted, not when soft-deleted. Preserve address data through soft-delete
and restore flows while retaining the existing cascade for permanent deletion.
In `@app-modules/identity/tests/Unit/User/UserPolicyTest.php`:
- Around line 47-61: Extend the UserPolicyTest mutation coverage by creating
recruiter and squad-captain users and asserting UserPolicy::update, delete,
restore, and forceDelete all return false for each read-only role. Preserve the
existing member and compliance assertions.
In
`@app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/ProfileSkillsRelationManager.php`:
- Around line 31-36: Add scoped uniqueness validation to the skill_id Select in
ProfileSkillsRelationManager, importing the required Unique rule and Profile
symbols. Configure the rule to constrain profile_id using the owner profile and
ignore the current record, so duplicate profile-skill assignments produce
Filament validation instead of a database exception.
In
`@app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/WorkExperiencesRelationManager.php`:
- Around line 46-56: Update the form schema in WorkExperiencesRelationManager so
is_currently_working_here is no longer live unless a dependent field is added,
and add validation ensuring end_date is not earlier than start_date and is
absent when the current-working checkbox is selected. Keep start_date required
and preserve the existing field labels.
In `@app-modules/panel-admin/src/Filament/Resources/Users/Schemas/UserForm.php`:
- Around line 58-61: Remove the unique validation from the name field in the
UserForm schema, specifically the TextInput::make('name') definition, while
preserving its label and required validation; keep uniqueness validation only on
the username field.
- Around line 81-95: Update the mutateRelationshipDataBeforeFillUsing callback
to verify that $data['preferences'] is an actual WorkPreferences instance before
accessing its properties. When the value is absent, null, or a plain array, use
a WorkPreferences default instance, and remove or correct the misleading `@var`
annotation while preserving the existing mapped fields.
---
Nitpick comments:
In
`@app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/WorkExperiencesRelationManager.php`:
- Around line 103-106: Replace the inline staff-role checks with the shared
UserPolicy ability check: update isEditableByCurrentUser() in
app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/WorkExperiencesRelationManager.php:103-106
and ProfileSkillsRelationManager.php:84-87 to use Gate::allows('update',
$this->getOwnerRecord()), and update the Moderação tab condition in
UserInfolist.php:49 to use the same ability. Ensure the required Gate import is
present in each affected file.
In
`@app-modules/panel-admin/src/Filament/Resources/Users/Schemas/UserInfolist.php`:
- Line 49: Extract the repeated `auth()->user()?->role->isStaff() ?? false`
check into a shared helper or gate, then update the visibility callback in the
UserInfolist and both relation managers to reuse it. Preserve the existing
boolean behavior for authenticated staff and all other users.
- Around line 366-369: Update the Voice query in UserInfolist to replace the
literal 'joined' state with the corresponding Voice state enum value, reusing
the existing enum symbol and leaving the query behavior unchanged.
In `@app-modules/panel-admin/src/Filament/Resources/Users/Tables/UsersTable.php`:
- Around line 113-116: Update the true and false callbacks in the queries
configuration to pass the boolean available_for_proposals values positionally to
whereRelation instead of using the operator named argument; preserve the
existing profile relationship and filtering behavior.
In `@app-modules/panel-admin/src/Filament/Resources/Users/UserResource.php`:
- Around line 77-80: Update UserResource::getGloballySearchableAttributes() to
remove email from the globally searchable attributes, retaining only username
and name so global search does not expose email values to panel roles.
In `@app-modules/panel-admin/tests/Feature/Users/UserResourceTest.php`:
- Around line 27-39: Replace the foreach in the “staff, recruiter e squad
captain listam os usuários” test with a Pest dataset that supplies each role to
a separate test case, preserving the existing viewer creation, authentication,
and table assertions so failures identify the specific role.
- Around line 90-128: Extend the user edit feature tests around EditUser to
cover the UserForm name uniqueness rule: create two users, edit one while
assigning the other’s name, submit the form, and assert the validation error is
present and the update is not accepted. Keep the existing successful multi-field
submission test unchanged.
- Around line 279-298: Expand the test “recrutador não vê ações de
criar/editar/excluir nas relation managers” by creating representative
WorkExperience and ProfileSkill records for the target, then assert the create,
edit, and delete table actions are hidden in both WorkExperiencesRelationManager
and ProfileSkillsRelationManager. Keep the existing authentication and page
setup unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ac38e745-698b-4962-b0e6-489690ffa43e
📒 Files selected for processing (21)
app-modules/identity/database/factories/UserFactory.phpapp-modules/identity/database/migrations/2026_07_26_120000_add_role_and_soft_deletes_to_users_table.phpapp-modules/identity/lang/en/enums.phpapp-modules/identity/lang/pt_BR/enums.phpapp-modules/identity/src/IdentityServiceProvider.phpapp-modules/identity/src/User/Enums/Role.phpapp-modules/identity/src/User/Models/User.phpapp-modules/identity/src/User/Policies/UserPolicy.phpapp-modules/identity/tests/Unit/User/UserPolicyTest.phpapp-modules/panel-admin/src/Filament/Resources/Users/Pages/EditUser.phpapp-modules/panel-admin/src/Filament/Resources/Users/Pages/ListUsers.phpapp-modules/panel-admin/src/Filament/Resources/Users/Pages/ViewUser.phpapp-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/ProfileSkillsRelationManager.phpapp-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/WorkExperiencesRelationManager.phpapp-modules/panel-admin/src/Filament/Resources/Users/Schemas/UserForm.phpapp-modules/panel-admin/src/Filament/Resources/Users/Schemas/UserInfolist.phpapp-modules/panel-admin/src/Filament/Resources/Users/Tables/UsersTable.phpapp-modules/panel-admin/src/Filament/Resources/Users/UserResource.phpapp-modules/panel-admin/src/PanelAdminServiceProvider.phpapp-modules/panel-admin/tests/Feature/Users/UserResourceTest.phpapp/Providers/AuthServiceProvider.php
Comment on lines
+32
to
+34
| Schema::table('users', static function (Blueprint $table): void { | ||
| $table->unique('username'); | ||
| $table->dropColumn(['role', 'deleted_at']); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make rollback safe after username reuse.
After up() permits an active and soft-deleted user to share a username, unique('username') will fail during rollback. Reconcile or rename deleted duplicates before restoring the global constraint.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app-modules/identity/database/migrations/2026_07_26_120000_add_role_and_soft_deletes_to_users_table.php`
around lines 32 - 34, Update the rollback logic in the users-table migration
before applying unique('username') to reconcile soft-deleted duplicate
usernames, such as by renaming or otherwise separating deleted duplicates; then
restore the global username constraint and remove the role and deleted_at
columns.
| use HasUuids; | ||
| use InteractsWithMedia; | ||
| use Notifiable; | ||
| use SoftDeletes; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not delete addresses on soft delete.
SoftDeletes triggers UserObserver::deleted() (app-modules/identity/src/User/Observers/UserObserver.php lines 17-19), which deletes the address. A restored user therefore loses address data. Restrict this cascade to force deletes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app-modules/identity/src/User/Models/User.php` at line 61, Update the User
model’s deletion behavior around SoftDeletes and UserObserver::deleted() so
address deletion occurs only when the user is force-deleted, not when
soft-deleted. Preserve address data through soft-delete and restore flows while
retaining the existing cascade for permanent deletion.
Comment on lines
+47
to
+61
| test('update and delete deny non-staff roles', function (): void { | ||
| $member = User::factory()->create(); | ||
|
|
||
| expect($this->policy->update($member))->toBeFalse() | ||
| ->and($this->policy->delete($member))->toBeFalse(); | ||
| }); | ||
|
|
||
| test('restore and forceDelete require compliance', function (): void { | ||
| $compliance = User::factory()->compliance()->create(); | ||
| $staff = User::factory()->staff()->create(); | ||
|
|
||
| expect($this->policy->restore($compliance))->toBeTrue() | ||
| ->and($this->policy->forceDelete($compliance))->toBeTrue() | ||
| ->and($this->policy->restore($staff))->toBeFalse() | ||
| ->and($this->policy->forceDelete($staff))->toBeFalse(); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert read-only roles cannot mutate.
Add recruiter and squad-captain denial assertions for update, delete, restore, and force-delete. The current tests only cover Member for non-staff mutation denial.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app-modules/identity/tests/Unit/User/UserPolicyTest.php` around lines 47 -
61, Extend the UserPolicyTest mutation coverage by creating recruiter and
squad-captain users and asserting UserPolicy::update, delete, restore, and
forceDelete all return false for each read-only role. Preserve the existing
member and compliance assertions.
Comment on lines
+31
to
+36
| Select::make('skill_id') | ||
| ->label('Skill') | ||
| ->relationship('skill', 'name') | ||
| ->searchable() | ||
| ->preload() | ||
| ->required(), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f -e php . --full-path '*migrations*' --exec rg -ln 'profile_skills' {} \; | xargs -r cat -nRepository: he4rt/heartdevs.com
Length of output: 257
🏁 Script executed:
#!/bin/bash
set -eu
echo "== candidate files =="
fd -t f -e php . | rg 'ProfileSkillsRelationManager|Migration.*Skills|ProfileSkill|skills.*profile|profile.*skills' || true
echo "== git status short =="
git status --short
echo "== find all php files with profile_skills =="
rg -n "profile_skills|ProfileSkill|profile_id|skill_id" --glob '*.php' . | head -200Repository: he4rt/heartdevs.com
Length of output: 15665
🏁 Script executed:
#!/bin/bash
set -eu
echo "== relation manager =="
cat -n app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/ProfileSkillsRelationManager.php | sed -n '1,140p'
echo "== migrations =="
for f in app-modules/profile/database/migrations/2026_07_07_000001_create_profile_skills_table.php app-modules/profile/database/migrations/2026_07_07_000000_create_skills_table.php; do
echo "--- $f"
cat -n "$f"
done
echo "== admin create/update/profile tests around profile skills =="
cat -n app-modules/panel-admin/tests/Feature/Users/UserResourceTest.php | sed -n '218,315p'Repository: he4rt/heartdevs.com
Length of output: 15678
Add a scoped unique validation for existing skills.
profile_skills.profile_id + skill_id is unique, but this RelationManager’s form only uses a relationship Select; recreating the same skill for the same profile can raise a raw database exception instead of showing a Filament validation error. Add ->unique(modifyRuleUsing: fn (Unique $rule) => $rule->where('profile_id', Profile::ensureExists($this->getOwnerRecord()->id)->id), ignoreRecord: true).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/ProfileSkillsRelationManager.php`
around lines 31 - 36, Add scoped uniqueness validation to the skill_id Select in
ProfileSkillsRelationManager, importing the required Unique rule and Profile
symbols. Configure the rule to constrain profile_id using the owner profile and
ignore the current record, so duplicate profile-skill assignments produce
Filament validation instead of a database exception.
Comment on lines
+46
to
+56
| DatePicker::make('start_date') | ||
| ->label('Start Date') | ||
| ->required(), | ||
|
|
||
| DatePicker::make('end_date') | ||
| ->label('End Date'), | ||
|
|
||
| Checkbox::make('is_currently_working_here') | ||
| ->label('Currently Working Here') | ||
| ->live(), | ||
| ]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
->live() has no dependents, and date consistency is unvalidated.
is_currently_working_here triggers re-renders for nothing, and nothing prevents end_date < start_date or an end date on a current job.
Proposed fix
DatePicker::make('start_date')
->label('Start Date')
- ->required(),
+ ->required()
+ ->maxDate(now()),
DatePicker::make('end_date')
- ->label('End Date'),
+ ->label('End Date')
+ ->afterOrEqual('start_date')
+ ->hidden(fn (Get $get): bool => (bool) $get('is_currently_working_here')),
Checkbox::make('is_currently_working_here')
->label('Currently Working Here')
->live(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| DatePicker::make('start_date') | |
| ->label('Start Date') | |
| ->required(), | |
| DatePicker::make('end_date') | |
| ->label('End Date'), | |
| Checkbox::make('is_currently_working_here') | |
| ->label('Currently Working Here') | |
| ->live(), | |
| ]); | |
| DatePicker::make('start_date') | |
| ->label('Start Date') | |
| ->required() | |
| ->maxDate(now()), | |
| DatePicker::make('end_date') | |
| ->label('End Date') | |
| ->afterOrEqual('start_date') | |
| ->hidden(fn (Get $get): bool => (bool) $get('is_currently_working_here')), | |
| Checkbox::make('is_currently_working_here') | |
| ->label('Currently Working Here') | |
| ->live(), | |
| ]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app-modules/panel-admin/src/Filament/Resources/Users/RelationManagers/WorkExperiencesRelationManager.php`
around lines 46 - 56, Update the form schema in WorkExperiencesRelationManager
so is_currently_working_here is no longer live unless a dependent field is
added, and add validation ensuring end_date is not earlier than start_date and
is absent when the current-working checkbox is selected. Keep start_date
required and preserve the existing field labels.
Comment on lines
+58
to
+61
| TextInput::make('name') | ||
| ->label('Name') | ||
| ->required() | ||
| ->unique(ignoreRecord: true), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
name should not be unique.
Display names legitimately collide; only username is a natural key. This blocks saving any user whose real name already exists.
Proposed fix
TextInput::make('name')
->label('Name')
- ->required()
- ->unique(ignoreRecord: true),
+ ->required(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| TextInput::make('name') | |
| ->label('Name') | |
| ->required() | |
| ->unique(ignoreRecord: true), | |
| TextInput::make('name') | |
| ->label('Name') | |
| ->required(), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app-modules/panel-admin/src/Filament/Resources/Users/Schemas/UserForm.php`
around lines 58 - 61, Remove the unique validation from the name field in the
UserForm schema, specifically the TextInput::make('name') definition, while
preserving its label and required validation; keep uniqueness validation only on
the username field.
Comment on lines
+81
to
+95
| ->mutateRelationshipDataBeforeFillUsing(function (array $data): array { | ||
| /** @var WorkPreferences $preferences */ | ||
| $preferences = $data['preferences'] ?? new WorkPreferences(); | ||
|
|
||
| return [ | ||
| ...Arr::except($data, ['preferences']), | ||
| 'has_disability' => $preferences->hasDisability, | ||
| 'willing_to_relocate' => $preferences->willingToRelocate, | ||
| 'is_open_to_remote' => $preferences->isOpenToRemote, | ||
| 'employment_types' => array_map( | ||
| static fn (EmploymentType $type): string => $type->value, | ||
| $preferences->employmentTypes, | ||
| ), | ||
| ]; | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
@var WorkPreferences annotation is wrong when the key is absent/null.
The docblock asserts the type before the null-coalesce; if preferences is a plain array (unhydrated cast) the property reads below fatal. Guard on the instance instead.
Proposed fix
- /** `@var` WorkPreferences $preferences */
- $preferences = $data['preferences'] ?? new WorkPreferences();
+ $preferences = $data['preferences'] instanceof WorkPreferences
+ ? $data['preferences']
+ : new WorkPreferences();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ->mutateRelationshipDataBeforeFillUsing(function (array $data): array { | |
| /** @var WorkPreferences $preferences */ | |
| $preferences = $data['preferences'] ?? new WorkPreferences(); | |
| return [ | |
| ...Arr::except($data, ['preferences']), | |
| 'has_disability' => $preferences->hasDisability, | |
| 'willing_to_relocate' => $preferences->willingToRelocate, | |
| 'is_open_to_remote' => $preferences->isOpenToRemote, | |
| 'employment_types' => array_map( | |
| static fn (EmploymentType $type): string => $type->value, | |
| $preferences->employmentTypes, | |
| ), | |
| ]; | |
| }) | |
| ->mutateRelationshipDataBeforeFillUsing(function (array $data): array { | |
| $preferences = $data['preferences'] instanceof WorkPreferences | |
| ? $data['preferences'] | |
| : new WorkPreferences(); | |
| return [ | |
| ...Arr::except($data, ['preferences']), | |
| 'has_disability' => $preferences->hasDisability, | |
| 'willing_to_relocate' => $preferences->willingToRelocate, | |
| 'is_open_to_remote' => $preferences->isOpenToRemote, | |
| 'employment_types' => array_map( | |
| static fn (EmploymentType $type): string => $type->value, | |
| $preferences->employmentTypes, | |
| ), | |
| ]; | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app-modules/panel-admin/src/Filament/Resources/Users/Schemas/UserForm.php`
around lines 81 - 95, Update the mutateRelationshipDataBeforeFillUsing callback
to verify that $data['preferences'] is an actual WorkPreferences instance before
accessing its properties. When the value is absent, null, or a plain array, use
a WorkPreferences default instance, and remove or correct the misleading `@var`
annotation while preserving the existing mapped fields.
Comment on lines
+58
to
+61
| TextInput::make('name') | ||
| ->label('Name') | ||
| ->required() | ||
| ->unique(ignoreRecord: true), |
Contributor
There was a problem hiding this comment.
Acho que aqui tem que se atentar por que pessoas podem ter o mesmo nome. kkk só pontuando isso por que coincidentemente eu soube de outra Bruna Domingues Leite na minha cidade sem nenhum parentesco comigo, então aqui deveria ser considerado.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #424
Contexto
Não existia
UserResourceno painel admin. Staff/moderação precisava de uma tela única pra ver e editar um membro por inteiro — os dados estavam espalhados entreCharacter,ExternalIdentity,Profile,AddresseModerationCase.Este PR entrega List, Edit e View para os dados editáveis pelo admin, mantendo como seções agregadas somente-leitura os dados que vêm de outros domínios (respeitando a fronteira presentation/core — sem duplicar lógica de escrita de outros módulos). Create ficou fora de escopo por decisão explícita no issue: contas só nascem via OAuth, e criação manual seria admitir débito técnico.
O que entra
Identity — hierarquia de roles (pré-requisito pro Policy do Resource)
Role(Staff,Compliance,Recruiter,SquadCaptain,Member) comisStaff(),isCompliance(),canViewUsers().rolee soft deletes aousers; o unique index deusernamevirou parcial (WHERE deleted_at IS NULL) pra não travar reuso de username por conta soft-deletada (regressão pega no fluxo de merge de contas).UserPolicy:viewAny/viewliberam staff/compliance/recruiter/squad captain;update/deleterestritos a staff;restore/forceDeleterestritos a compliance — hard delete nunca é o padrão, sempre com a confirmação explícita que o Filament já exige.profile(),workExperiences(),profileSkills()noUser(aceitável no core por poder ser reaproveitado por outros módulos, conforme discutido no issue).Panel-admin —
UserResource[25, 50, 100]; filtros de role, senioridade e disponibilidade.character()) — 100% somente-leitura, sem action de conceder badge (adiado pro issue de badges por evento, já combinado na thread).providers()).Decisões registradas durante a implementação
profileSkills()(registrosProfileSkilldiretos) em vez deProfile::skills()(BelongsToMany) — mesmo resultado prático, menos retrabalho sobre o que já existia.preferencesdo perfil é um cast custom (AsWorkPreferences), não array puro — o form usa os hooks nativos do Filament (mutateRelationshipDataBeforeFill/SaveUsing) pra achatar/reagrupar os campos, seguindo o mesmo padrão manual já usado emProfilePage(portal).Testes
UserPolicyTest: cobre a hierarquia de roles em todas as abilities.UserResourceTest(18 testes): autorização por página/seção (List/Edit/View × staff/compliance/recruiter/squad captain/member), edição multi-seção com persistência depreferences/social_links/endereço, validação de username duplicado, estado vazio pra usuário semCharacter, RelationManagers (create/edit/delete pra staff, ocultas pra recruiter), soft delete como ação padrão, hard delete restrito a compliance.PHPStan (nível 6), Pint e Rector limpos nos arquivos tocados.
Como testar manualmente
make dev, logar em/admincom uma contastaffoucompliance.compliance, a exclusão física (hard delete, com confirmação).