fix(api): fix 404 error on successful profile update#83
Conversation
- Remove problematic logic in EndpointExtensions that returned 404 for successful responses with null data. - Update UpdateProfileHandler to return success message in the Data field.
📝 WalkthroughWalkthroughTwo presentation-layer changes address response handling: EndpointExtensions.ToActionResult now always returns ChangesService Response Handling Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 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📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🤖 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
`@backend/Src/PIED_LMS.Application/UserCases/Commands/Identity/UpdateProfileHandler.cs`:
- Line 66: The ServiceResponse<string> returned in UpdateProfileHandler is using
the same message for both Message and Data; replace the redundant Data value
with a meaningful payload (preferably return the updated user DTO from your
updated model, e.g., UpdatedUserDto or UserProfileDto) or switch the response
type to ServiceResponse<bool> and return true as Data; update the return in
UpdateProfileHandler.Handle to construct ServiceResponse with the chosen Data
(e.g., the updated user DTO or a boolean) instead of the duplicate "Profile
updated successfully" string.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 24c0d2b2-4a70-42e0-9746-ad68365ee9f5
📒 Files selected for processing (2)
backend/Src/PIED_LMS.Application/UserCases/Commands/Identity/UpdateProfileHandler.csbackend/Src/PIED_LMS.Presentation/Extensions/EndpointExtensions.cs
| } | ||
|
|
||
| return new ServiceResponse<string>(true, "Profile updated successfully"); | ||
| return new ServiceResponse<string>(true, "Profile updated successfully", "Profile updated successfully"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify ServiceResponse constructor signatures and usage patterns
# Find ServiceResponse class definition
echo "=== ServiceResponse class definition ==="
ast-grep --pattern 'class ServiceResponse<$_> {
$$$
}'
# Find constructor definitions
echo -e "\n=== ServiceResponse constructors ==="
rg -n -A 5 'class ServiceResponse' --type cs
# Find other usages of 3-parameter ServiceResponse constructor
echo -e "\n=== Other usages of 3-parameter ServiceResponse<string> constructor ==="
rg -n 'new ServiceResponse<string>\([^,]+,[^,]+,[^)]+\)' --type cs -C 2Repository: PiedTeam/pied-lms
Length of output: 9443
🏁 Script executed:
find . -name "*.cs" -type f | head -20Repository: PiedTeam/pied-lms
Length of output: 1357
🏁 Script executed:
rg -l "class ServiceResponse" --type csRepository: PiedTeam/pied-lms
Length of output: 43
🏁 Script executed:
fd -e cs -x grep -l "class ServiceResponse" {} \;Repository: PiedTeam/pied-lms
Length of output: 43
🏁 Script executed:
find ./backend -name "*.cs" -type f -exec grep -l "class ServiceResponse" {} \;Repository: PiedTeam/pied-lms
Length of output: 43
🏁 Script executed:
grep -r "class ServiceResponse" ./backend --include="*.cs" -nRepository: PiedTeam/pied-lms
Length of output: 43
🏁 Script executed:
grep -r "using.*ServiceResponse\|ServiceResponse.*namespace" ./backend --include="*.cs" -B 2 -A 2Repository: PiedTeam/pied-lms
Length of output: 43
🏁 Script executed:
grep -r "ServiceResponse<" ./backend --include="*.cs" | head -20Repository: PiedTeam/pied-lms
Length of output: 3485
🏁 Script executed:
cat ./backend/Src/PIED_LMS.Application/UserCases/Commands/Identity/UpdateProfileHandler.cs | head -80Repository: PiedTeam/pied-lms
Length of output: 3119
🏁 Script executed:
find ./backend -type d -name "*Contract*" -o -name "*Shared*" -o -name "*Common*" | head -20Repository: PiedTeam/pied-lms
Length of output: 145
🏁 Script executed:
find ./backend -name "*.cs" -type f | xargs grep -l "class ServiceResponse" 2>/dev/nullRepository: PiedTeam/pied-lms
Length of output: 45
🏁 Script executed:
find . -path "*/Contract/*" -name "*.cs" -type f | head -30Repository: PiedTeam/pied-lms
Length of output: 43
🏁 Script executed:
find ./backend/Src/PIED_LMS.Contract -name "*.cs" -type f | xargs grep -l "ServiceResponse"Repository: PiedTeam/pied-lms
Length of output: 1774
🏁 Script executed:
ls -la ./backend/Src/PIED_LMS.Contract/Repository: PiedTeam/pied-lms
Length of output: 621
🏁 Script executed:
find ./backend/Src/PIED_LMS.Contract -name "*.cs" -type f -exec grep -l "class ServiceResponse" {} \;Repository: PiedTeam/pied-lms
Length of output: 43
🏁 Script executed:
find ./backend/Src/PIED_LMS.Contract/Abstractions -name "*.cs" -type fRepository: PiedTeam/pied-lms
Length of output: 908
🏁 Script executed:
cat ./backend/Src/PIED_LMS.Contract/GlobalUsings.csRepository: PiedTeam/pied-lms
Length of output: 235
🏁 Script executed:
grep -r "ServiceResponse" ./backend/Src/PIED_LMS.Contract/Abstractions --include="*.cs" -nRepository: PiedTeam/pied-lms
Length of output: 197
🏁 Script executed:
cat ./backend/Src/PIED_LMS.Contract/Abstractions/Shared/ResultT.csRepository: PiedTeam/pied-lms
Length of output: 706
🏁 Script executed:
cat ./backend/Src/PIED_LMS.Contract/Abstractions/Shared/Result.csRepository: PiedTeam/pied-lms
Length of output: 1165
🏁 Script executed:
grep -r "ServiceResponse\|using.*Result\|alias" ./backend/Src/PIED_LMS.Contract --include="*.cs" -n | head -30Repository: PiedTeam/pied-lms
Length of output: 3680
🏁 Script executed:
grep -r "class ServiceResponse\|record ServiceResponse" ./backend --include="*.cs"Repository: PiedTeam/pied-lms
Length of output: 157
🏁 Script executed:
find ./backend -name "*.csproj" -type f -exec cat {} \; | grep -i "serviceresponse\|package"Repository: PiedTeam/pied-lms
Length of output: 2465
🏁 Script executed:
rg "class ServiceResponse|namespace.*ServiceResponse" ./backend --type cs -iRepository: PiedTeam/pied-lms
Length of output: 43
🏁 Script executed:
cat ./backend/Src/PIED_LMS.Contract/Services/Identity/Response.csRepository: PiedTeam/pied-lms
Length of output: 1427
Remove the redundant duplicate string in the Data parameter.
The ServiceResponse<T> constructor accepts a Data field to carry meaningful information, yet you're passing "Profile updated successfully" as both the Message and Data parameters. This duplication is wasteful and defeats the purpose of the Data field.
Choose one approach:
- Return the updated user profile data as a DTO
- Return a specific success indicator (e.g., the user ID or updated timestamp)
- Use a simpler response type like
ServiceResponse<bool>withtrueas the data 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
`@backend/Src/PIED_LMS.Application/UserCases/Commands/Identity/UpdateProfileHandler.cs`
at line 66, The ServiceResponse<string> returned in UpdateProfileHandler is
using the same message for both Message and Data; replace the redundant Data
value with a meaningful payload (preferably return the updated user DTO from
your updated model, e.g., UpdatedUserDto or UserProfileDto) or switch the
response type to ServiceResponse<bool> and return true as Data; update the
return in UpdateProfileHandler.Handle to construct ServiceResponse with the
chosen Data (e.g., the updated user DTO or a boolean) instead of the duplicate
"Profile updated successfully" string.
1 participant
Summary by CodeRabbit