feat(profile): UpsertProfile and ToggleAvailability actions#283
Closed
Hadzab wants to merge 1 commit into
Closed
Conversation
📝 WalkthroughWalkthroughThis pull request introduces two profile management actions and their supporting infrastructure. It adds 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches⚔️ Resolve merge conflicts
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: 2
🧹 Nitpick comments (2)
app-modules/profile/src/Actions/ToggleAvailability.php (1)
15-15: ⚡ Quick winSimplify validation and localize error message.
Two improvements for this validation:
The
instanceofcheck is redundant given the?StartAvailabilitytype hint. If$startAvailabilityis not null, it must be aStartAvailabilityinstance. Simplify to a null check.The error message is hardcoded in Portuguese. Use Laravel's localization helpers for i18n support.
♻️ Proposed refactor
-throw_if($available && !$startAvailability instanceof StartAvailability, InvalidArgumentException::class, 'O prazo de disponibilidade é obrigatório ao ativar a disponibilidade.'); +throw_if($available && $startAvailability === null, InvalidArgumentException::class, __('profile::validation.start_availability_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/profile/src/Actions/ToggleAvailability.php` at line 15, In ToggleAvailability.php update the validation throw to use a null check instead of instanceof and localize the error string: replace the condition "throw_if($available && !$startAvailability instanceof StartAvailability, ... , '...');" with a check like "throw_if($available && is_null($startAvailability), InvalidArgumentException::class, __('profile.availability.start_required'))" (use StartAvailability and $startAvailability to locate the code and add a suitable translation key in your language files).app-modules/profile/tests/Feature/UpsertProfileTest.php (1)
57-100: ⚡ Quick winAdd regression tests for uncovered nullable/length contract cases.
Please add negative coverage for
nicknamelength (>100) and one scenario asserting explicitnullclears a nullable field (once DTO/action semantics are updated).Example tests
+test('rejeita nickname acima de 100 caracteres', function (): void { + $profile = Profile::factory()->create(); + $dto = UpsertProfileDTO::fromArray(['nickname' => str_repeat('a', 101)]); + + expect(fn () => resolve(UpsertProfile::class)->handle($profile, $dto)) + ->toThrow(InvalidArgumentException::class); +}); + +test('permite limpar campo nullable com null explicito', function (): void { + $profile = Profile::factory()->create(['headline' => 'Old Headline']); + $dto = UpsertProfileDTO::fromArray(['headline' => null]); + + $updated = resolve(UpsertProfile::class)->handle($profile, $dto); + expect($updated->headline)->toBeNull(); +});🤖 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/profile/tests/Feature/UpsertProfileTest.php` around lines 57 - 100, Add two tests to UpsertProfileTest mirroring existing patterns: one that creates a Profile, builds an UpsertProfileDTO with 'nickname' => str_repeat('a', 101) and asserts resolve(UpsertProfile::class)->handle($profile, $dto) throws InvalidArgumentException (same style as the headline/bio length tests), and a second that creates a Profile with an existing nickname, calls UpsertProfileDTO::fromArray(['nickname' => null]) and asserts the returned $updated from resolve(UpsertProfile::class)->handle($profile, $dto) has ->nickname === null (nullable-clear assertion, matching the social_links/years_experience test patterns); use the same expect(...) helpers and exception/type checks as in the 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/profile/src/Actions/UpsertProfile.php`:
- Around line 33-40: The validate method is missing a max-length check for
UpsertProfileDTO::nickname; add a throw_if similar to the others inside
validate() that checks $dto->nickname !== null && mb_strlen($dto->nickname) >
100 and throws InvalidArgumentException with a clear Portuguese message (e.g. 'O
apelido não pode ultrapassar 100 caracteres.') so nickname cannot exceed 100
chars.
In `@app-modules/profile/src/DTOs/UpsertProfileDTO.php`:
- Around line 26-35: The DTO factory method fromArray currently collapses
missing keys and explicit nulls into the same property values (see fromArray and
fields like nickname, about, headline, socialLinks, seniorityLevel), so updates
cannot clear values; change fromArray to record which keys were explicitly
provided (e.g. populate a $provided array/property on the DTO for keys present
in the input) and preserve explicit nulls (use isset/array_key_exists checks
when building the DTO, calling Date::parse or SeniorityLevel::from only when the
key exists). Then update He4rt\Profile\Actions\UpsertProfile to filter updates
by the DTO’s provided keys (the new $dto->provided) instead of !is_null($value)
so explicit nulls can be applied to clear fields.
---
Nitpick comments:
In `@app-modules/profile/src/Actions/ToggleAvailability.php`:
- Line 15: In ToggleAvailability.php update the validation throw to use a null
check instead of instanceof and localize the error string: replace the condition
"throw_if($available && !$startAvailability instanceof StartAvailability, ... ,
'...');" with a check like "throw_if($available && is_null($startAvailability),
InvalidArgumentException::class, __('profile.availability.start_required'))"
(use StartAvailability and $startAvailability to locate the code and add a
suitable translation key in your language files).
In `@app-modules/profile/tests/Feature/UpsertProfileTest.php`:
- Around line 57-100: Add two tests to UpsertProfileTest mirroring existing
patterns: one that creates a Profile, builds an UpsertProfileDTO with 'nickname'
=> str_repeat('a', 101) and asserts
resolve(UpsertProfile::class)->handle($profile, $dto) throws
InvalidArgumentException (same style as the headline/bio length tests), and a
second that creates a Profile with an existing nickname, calls
UpsertProfileDTO::fromArray(['nickname' => null]) and asserts the returned
$updated from resolve(UpsertProfile::class)->handle($profile, $dto) has
->nickname === null (nullable-clear assertion, matching the
social_links/years_experience test patterns); use the same expect(...) helpers
and exception/type checks as in the file.
🪄 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
Run ID: 584ef9fd-ad63-4e1f-bf61-91e8e565378a
📒 Files selected for processing (4)
app-modules/profile/src/Actions/ToggleAvailability.phpapp-modules/profile/src/Actions/UpsertProfile.phpapp-modules/profile/src/DTOs/UpsertProfileDTO.phpapp-modules/profile/tests/Feature/UpsertProfileTest.php
Comment on lines
+33
to
+40
| private function validate(UpsertProfileDTO $dto): void | ||
| { | ||
| throw_if($dto->about !== null && mb_strlen($dto->about) > 500, InvalidArgumentException::class, 'O campo "sobre" não pode ultrapassar 500 caracteres.'); | ||
|
|
||
| throw_if($dto->headline !== null && mb_strlen($dto->headline) > 100, InvalidArgumentException::class, 'O título não pode ultrapassar 100 caracteres.'); | ||
|
|
||
| throw_if($dto->yearsExperience !== null && ($dto->yearsExperience < 0 || $dto->yearsExperience > 50), InvalidArgumentException::class, 'Os anos de experiência devem estar entre 0 e 50.'); | ||
|
|
There was a problem hiding this comment.
nickname max-length validation is missing.
Line 35-40 validates about, headline, and yearsExperience, but not nickname (required max 100). This allows invalid profile data through this domain action.
Patch suggestion
private function validate(UpsertProfileDTO $dto): void
{
+ throw_if($dto->nickname !== null && mb_strlen($dto->nickname) > 100, InvalidArgumentException::class, 'O apelido não pode ultrapassar 100 caracteres.');
+
throw_if($dto->about !== null && mb_strlen($dto->about) > 500, InvalidArgumentException::class, 'O campo "sobre" não pode ultrapassar 500 caracteres.');📝 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
| private function validate(UpsertProfileDTO $dto): void | |
| { | |
| throw_if($dto->about !== null && mb_strlen($dto->about) > 500, InvalidArgumentException::class, 'O campo "sobre" não pode ultrapassar 500 caracteres.'); | |
| throw_if($dto->headline !== null && mb_strlen($dto->headline) > 100, InvalidArgumentException::class, 'O título não pode ultrapassar 100 caracteres.'); | |
| throw_if($dto->yearsExperience !== null && ($dto->yearsExperience < 0 || $dto->yearsExperience > 50), InvalidArgumentException::class, 'Os anos de experiência devem estar entre 0 e 50.'); | |
| private function validate(UpsertProfileDTO $dto): void | |
| { | |
| throw_if($dto->nickname !== null && mb_strlen($dto->nickname) > 100, InvalidArgumentException::class, 'O apelido não pode ultrapassar 100 caracteres.'); | |
| throw_if($dto->about !== null && mb_strlen($dto->about) > 500, InvalidArgumentException::class, 'O campo "sobre" não pode ultrapassar 500 caracteres.'); | |
| throw_if($dto->headline !== null && mb_strlen($dto->headline) > 100, InvalidArgumentException::class, 'O título não pode ultrapassar 100 caracteres.'); | |
| throw_if($dto->yearsExperience !== null && ($dto->yearsExperience < 0 || $dto->yearsExperience > 50), InvalidArgumentException::class, 'Os anos de experiência devem estar entre 0 e 50.'); | |
🤖 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/profile/src/Actions/UpsertProfile.php` around lines 33 - 40, The
validate method is missing a max-length check for UpsertProfileDTO::nickname;
add a throw_if similar to the others inside validate() that checks
$dto->nickname !== null && mb_strlen($dto->nickname) > 100 and throws
InvalidArgumentException with a clear Portuguese message (e.g. 'O apelido não
pode ultrapassar 100 caracteres.') so nickname cannot exceed 100 chars.
Comment on lines
+26
to
+35
| public static function fromArray(array $data): self | ||
| { | ||
| return new self( | ||
| nickname: $data['nickname'] ?? null, | ||
| birthdate: isset($data['birthdate']) ? Date::parse($data['birthdate']) : null, | ||
| about: $data['about'] ?? null, | ||
| headline: $data['headline'] ?? null, | ||
| seniorityLevel: isset($data['seniority_level']) ? SeniorityLevel::from($data['seniority_level']) : null, | ||
| yearsExperience: $data['years_experience'] ?? null, | ||
| socialLinks: $data['social_links'] ?? null, |
There was a problem hiding this comment.
Explicit null updates are not representable in the DTO mapping.
On Line 29-35, missing keys and explicitly provided null values collapse to the same DTO value. Combined with null filtering in the action, nullable fields cannot be cleared (for example about, headline, nickname, social_links).
Proposed direction
final readonly class UpsertProfileDTO
{
public function __construct(
public ?string $nickname = null,
public ?Carbon $birthdate = null,
public ?string $about = null,
public ?string $headline = null,
public ?SeniorityLevel $seniorityLevel = null,
public ?int $yearsExperience = null,
public ?array $socialLinks = null,
+ /** `@var` array<string, bool> */
+ public array $provided = [],
) {}
public static function fromArray(array $data): self
{
return new self(
- nickname: $data['nickname'] ?? null,
- birthdate: isset($data['birthdate']) ? Date::parse($data['birthdate']) : null,
- about: $data['about'] ?? null,
- headline: $data['headline'] ?? null,
- seniorityLevel: isset($data['seniority_level']) ? SeniorityLevel::from($data['seniority_level']) : null,
- yearsExperience: $data['years_experience'] ?? null,
- socialLinks: $data['social_links'] ?? null,
+ nickname: array_key_exists('nickname', $data) ? $data['nickname'] : null,
+ birthdate: array_key_exists('birthdate', $data) ? Date::parse($data['birthdate']) : null,
+ about: array_key_exists('about', $data) ? $data['about'] : null,
+ headline: array_key_exists('headline', $data) ? $data['headline'] : null,
+ seniorityLevel: array_key_exists('seniority_level', $data) ? SeniorityLevel::from($data['seniority_level']) : null,
+ yearsExperience: array_key_exists('years_experience', $data) ? $data['years_experience'] : null,
+ socialLinks: array_key_exists('social_links', $data) ? $data['social_links'] : null,
+ provided: array_fill_keys(array_keys($data), true),
);
}
}Then in He4rt\Profile\Actions\UpsertProfile, filter updates by $dto->provided keys instead of !is_null($value).
🤖 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/profile/src/DTOs/UpsertProfileDTO.php` around lines 26 - 35, The
DTO factory method fromArray currently collapses missing keys and explicit nulls
into the same property values (see fromArray and fields like nickname, about,
headline, socialLinks, seniorityLevel), so updates cannot clear values; change
fromArray to record which keys were explicitly provided (e.g. populate a
$provided array/property on the DTO for keys present in the input) and preserve
explicit nulls (use isset/array_key_exists checks when building the DTO, calling
Date::parse or SeniorityLevel::from only when the key exists). Then update
He4rt\Profile\Actions\UpsertProfile to filter updates by the DTO’s provided keys
(the new $dto->provided) instead of !is_null($value) so explicit nulls can be
applied to clear fields.
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
1 participant
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 #253
What was done
Implements the domain logic of the Profile module with two distinct actions and a DTO:
UpsertProfileDTO— DTO with the editable profile fields (nickname, birthdate, about, headline, seniorityLevel, yearsExperience, socialLinks) with afromArray()factory method.UpsertProfile— Action that receives an existing Profile and a DTO, validates the fields and performs a pure update. Validatesabout(max 500),headline(max 100),years_experience(0-50) andsocial_linksagainst theSocialPlatformenum.ToggleAvailability— Atomic action that toggles availability for proposals. When enabled,start_availabilityis required. When disabled, preserves the previous value.Tests
11 feature tests covering all BDD scenarios from the issue:
Files created
app-modules/profile/src/Actions/UpsertProfile.phpapp-modules/profile/src/Actions/ToggleAvailability.phpapp-modules/profile/src/DTOs/UpsertProfileDTO.phpapp-modules/profile/tests/Feature/UpsertProfileTest.php.phpDescription
Implements domain logic for the Profile module by introducing UpsertProfile and ToggleAvailability actions to replace legacy Identity-based profile update flows. The PR adds a DTO with factory method for profile editing, two domain actions with comprehensive validation rules, and 11 feature tests covering all BDD scenarios. Resolves issue
#253.References
#253: Profile domain actionsDependencies & Requirements
No new external dependencies are introduced. The implementation uses existing Laravel/Illuminate facades and the SeniorityLevel and SocialPlatform enums already present in the codebase.
Contributor Summary
Changes Summary
app-modules/profile/src/DTOs/UpsertProfileDTO.phpapp-modules/profile/src/Actions/UpsertProfile.phpapp-modules/profile/src/Actions/ToggleAvailability.phpapp-modules/profile/tests/Feature/UpsertProfileTest.php