feat: add GET, PUT and DELETE endpoints to PetController#150
feat: add GET, PUT and DELETE endpoints to PetController#150lindaeskilsson merged 2 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPetController was expanded with full CRUD endpoints (GET by ID, GET by owner, PUT, DELETE), now depends on UserService, and uses a new private Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as PetController
participant UserSvc as UserService
participant PetSvc as PetService
participant DB as Database
Client->>Controller: GET /pets/{petId} (currentUserId, petId)
Controller->>UserSvc: getUserEntityById(currentUserId)
UserSvc-->>Controller: User entity
Controller->>PetSvc: getPetById(petId, currentUser)
PetSvc->>DB: query pet by id & authorization checks
DB-->>PetSvc: Pet entity
PetSvc-->>Controller: Pet entity
Controller->>Client: 200 OK (PetResponse)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/org/example/vet1177/controller/PetController.java (1)
30-37: Pick one boundary for resolving the current user.
createPet,getPetsByOwner, andupdatePetpasscurrentUserIdintoPetService, whilegetPetByIdanddeletePetresolve aUserin the controller first. That splits not-found/authorization behavior across layers and makes the controller harder to keep consistent. I’d standardize on one approach for all five endpoints.Also applies to: 42-47, 53-57, 66-72, 77-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/vet1177/controller/PetController.java` around lines 30 - 37, The controller mixes two patterns for resolving the current user: some endpoints (createPet, getPetsByOwner, updatePet) forward a currentUserId to PetService while others (getPetById, deletePet) resolve a User in the controller; pick one boundary and apply it to all endpoints to centralize not-found/auth behavior. Either (A) resolve and validate the User in the controller for all endpoints and pass the User (or its ID known-valid) into PetService, or (B) accept only the raw currentUserId in controllers and move all User lookup/authorization into PetService; then update createPet, getPetsByOwner, updatePet, getPetById, and deletePet accordingly so they all follow the chosen pattern and remove duplicated lookup/authorization code between controller and PetService.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/example/vet1177/controller/PetController.java`:
- Around line 42-48: PetService methods (notably getPetById and deletePet) are
currently throwing raw RuntimeException; change those throws to the appropriate
typed exceptions so the GlobalExceptionHandler maps to correct HTTP statuses —
throw ResourceNotFoundException when the pet or related resource is missing,
throw ForbiddenException when the current user is not authorized to access or
delete the pet, and throw BusinessRuleException for validation/business-rule
violations; update the throw sites in PetService (replace RuntimeException
occurrences around the reported locations) so callers like PetController.receive
the typed exceptions.
---
Nitpick comments:
In `@src/main/java/org/example/vet1177/controller/PetController.java`:
- Around line 30-37: The controller mixes two patterns for resolving the current
user: some endpoints (createPet, getPetsByOwner, updatePet) forward a
currentUserId to PetService while others (getPetById, deletePet) resolve a User
in the controller; pick one boundary and apply it to all endpoints to centralize
not-found/auth behavior. Either (A) resolve and validate the User in the
controller for all endpoints and pass the User (or its ID known-valid) into
PetService, or (B) accept only the raw currentUserId in controllers and move all
User lookup/authorization into PetService; then update createPet,
getPetsByOwner, updatePet, getPetById, and deletePet accordingly so they all
follow the chosen pattern and remove duplicated lookup/authorization code
between controller and PetService.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 62bcd0ce-246d-4d34-950b-c0b72e76f59c
📒 Files selected for processing (1)
src/main/java/org/example/vet1177/controller/PetController.java
Closes #146
Vad är gjort
Lagt till saknade endpoints i PetController:
Ändringar
PetController.java– nya endpoints tillagdaUserServiceinjiceras för att lösa uppcurrentUserIdtillUser-entitettoResponse()-hjälpmetod extraherad för att undvika kodupprepningSummary by CodeRabbit
New Features
Bug Fixes