Add Incident module: entity, repository, service, controller, DTOs; r…#27
Conversation
…eorganize packages
|
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 (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughReorganized user-related classes into Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant IncidentController as "IncidentController"
participant IncidentService as "IncidentService"
participant SecurityContext as "SecurityContext"
participant IncidentRepository as "IncidentRepository"
participant Database as "Database"
Client->>IncidentController: POST /api/incidents (IncidentRequest)
IncidentController->>IncidentService: createIncident(Incident)
IncidentService->>SecurityContext: get Authentication / principal
SecurityContext-->>IncidentService: CustomUserDetails -> AppUser
IncidentService->>IncidentService: set createdBy, status=OPEN, createdAt=now
IncidentService->>IncidentRepository: save(Incident)
IncidentRepository->>Database: INSERT Incident
Database-->>IncidentRepository: persisted Incident
IncidentRepository-->>IncidentService: Incident
IncidentService-->>IncidentController: Incident
IncidentController->>Client: IncidentResponse (201)
Client->>IncidentController: GET /api/incidents
IncidentController->>IncidentService: findAll()
IncidentService->>IncidentRepository: findAll()
IncidentRepository->>Database: SELECT * FROM incidents
Database-->>IncidentRepository: List<Incident>
IncidentRepository-->>IncidentService: List<Incident>
IncidentService-->>IncidentController: List<Incident>
IncidentController->>Client: List<IncidentResponse> (200)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 (8)
src/main/java/org/example/team6backend/incident/service/IncidentService.java (2)
24-27: Add defensive check for principal type.While the OAuth2 flow currently guarantees
CustomUserDetails, a direct cast without validation can fail if authentication configuration changes (e.g., adding Basic Auth, API keys). Consider a defensive check:🛡️ Proposed defensive check
public Incident createIncident(Incident incident){ Authentication auth = SecurityContextHolder.getContext().getAuthentication(); - CustomUserDetails userDetails = (CustomUserDetails) auth.getPrincipal(); + Object principal = auth.getPrincipal(); + if (!(principal instanceof CustomUserDetails userDetails)) { + throw new IllegalStateException("Unexpected principal type: " + principal.getClass()); + } AppUser appUser = userDetails.getUser();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/incident/service/IncidentService.java` around lines 24 - 27, The createIncident method currently casts Authentication.getPrincipal() directly to CustomUserDetails which can fail if the principal type changes; update the method to first retrieve Authentication from SecurityContextHolder, check that auth.getPrincipal() is an instance of CustomUserDetails (using instanceof), then cast and proceed to get AppUser; if the principal is not CustomUserDetails, handle gracefully (throw a clear unchecked exception or return a 401/forbidden result) so createIncident, Authentication, SecurityContextHolder, CustomUserDetails and AppUser are guarded against configuration changes.
45-46: Minor formatting inconsistency.Space between
Listand<Incident>.✏️ Fix formatting
- public List <Incident> findAll(){ + public List<Incident> findAll() { return incidentRepository.findAll(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/incident/service/IncidentService.java` around lines 45 - 46, The method signature in IncidentService.findAll has a formatting inconsistency: remove the extra space between the generic type and the raw type so it reads List<Incident> findAll() instead of List <Incident> findAll(); update the method declaration in the IncidentService class accordingly (keeping return incidentRepository.findAll() unchanged).src/main/java/org/example/team6backend/incident/repository/IncidentRepository.java (1)
11-12: Minor formatting inconsistency.Line 12 has a space between
Listand<Incident>while line 11 doesn't.✏️ Fix formatting
List<Incident> findByCreatedBy(AppUser user); - List <Incident> findByAssignedTo(AppUser user); + List<Incident> findByAssignedTo(AppUser user);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/incident/repository/IncidentRepository.java` around lines 11 - 12, Fix the minor formatting inconsistency in the repository interface: in the method declaration for findByAssignedTo(AppUser user) (the symbol to change is the return type on the line containing findByAssignedTo), remove the unintended space between "List" and "<Incident>" so both method signatures use the same "List<Incident>" formatting as findByCreatedBy(AppUser user).src/main/java/org/example/team6backend/incident/entity/Incident.java (3)
26-27: Consider addingmodifiedAttimestamp for audit consistency.The entity has
modifiedBybut lacks a correspondingmodifiedAttimestamp. For complete audit trails, both fields are typically tracked together.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/incident/entity/Incident.java` around lines 26 - 27, The Incident entity has a modifiedBy field but no modifiedAt timestamp; add a new field (e.g., private Instant modifiedAt or LocalDateTime modifiedAt) to the Incident class to record when modifications occur, annotate it appropriately for persistence (e.g., `@Column` and/or `@LastModifiedDate/`@Temporal as used elsewhere in the project), update constructors/getters/setters and any builders or DTO mappers that touch Incident, and ensure the service/repository layer that sets modifiedBy also sets modifiedAt so the audit trail remains consistent with the modifiedBy field.
23-30: Consider lazy fetching for@ManyToOnerelationships.The default fetch type for
@ManyToOneisEAGER, which loads all threeAppUserassociations immediately. When fetching lists of incidents (e.g.,findAll()), this can result in additional queries per incident.For list operations, consider using
FetchType.LAZYand fetching users only when needed:♻️ Proposed change
- `@ManyToOne` + `@ManyToOne`(fetch = FetchType.LAZY) private AppUser createdBy; - `@ManyToOne` + `@ManyToOne`(fetch = FetchType.LAZY) private AppUser modifiedBy; - `@ManyToOne` + `@ManyToOne`(fetch = FetchType.LAZY) private AppUser assignedTo;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/incident/entity/Incident.java` around lines 23 - 30, Update the three `@ManyToOne` associations in the Incident entity (fields createdBy, modifiedBy, assignedTo) to use lazy loading by adding fetch = FetchType.LAZY to each annotation; ensure you import javax.persistence.FetchType. Keep business logic that expects user data working by explicitly fetching users when needed (e.g., use JOIN FETCH in repository queries or map to DTOs) to avoid LazyInitializationException at runtime.
8-9: Consider using Lombok for consistency with other entities.
AppUseruses Lombok@Data. This entity has manual getters/setters, which adds boilerplate. Consider using Lombok for consistency:♻️ Proposed change
package org.example.team6backend.incident.entity; import jakarta.persistence.*; +import lombok.Data; +import lombok.NoArgsConstructor; import org.example.team6backend.user.entity.AppUser; import java.time.LocalDateTime; `@Entity` +@Data +@NoArgsConstructor public class Incident { `@Id` `@GeneratedValue`(strategy = GenerationType.IDENTITY) private Long id; // ... fields only, remove manual getters/setters🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/incident/entity/Incident.java` around lines 8 - 9, The Incident entity currently defines manual getters/setters while other entities (e.g., AppUser) use Lombok; replace the boilerplate by adding Lombok annotations to the Incident class (for example `@Data` plus `@NoArgsConstructor` and `@AllArgsConstructor` or `@Builder` as needed), add the corresponding lombok imports, and remove the explicit getter/setter methods so the class relies on Lombok-generated accessors; ensure the class-level annotation is placed on class Incident and run a compile to confirm no remaining manually defined accessors conflict.src/main/java/org/example/team6backend/incident/controller/IncidentController.java (2)
32-38: Consider adding pagination for scalability.Returning all incidents without pagination can cause performance issues and high memory consumption as the dataset grows. Spring Data's
Pageablesupport makes this straightforward to implement.♻️ Proposed pagination approach
+import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable;`@GetMapping` -public List<IncidentResponse> getAllIncidents() { - return incidentService.findAll() - .stream() - .map(IncidentResponse::fromEntity) - .toList(); +public Page<IncidentResponse> getAllIncidents(Pageable pageable) { + return incidentService.findAll(pageable) + .map(IncidentResponse::fromEntity); }This requires updating
IncidentService.findAll()to accept aPageableparameter and returnPage<Incident>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/incident/controller/IncidentController.java` around lines 32 - 38, Modify getAllIncidents to accept Spring Pageable (or `@RequestParam` page/size and build one) and call incidentService.findAll(pageable) instead of the parameterless findAll(); update IncidentService.findAll(Pageable) to return Page<Incident>, then map the Page content via IncidentResponse::fromEntity and return a paged response (e.g., Page<IncidentResponse> or ResponseEntity<Page<IncidentResponse>>). Ensure you change the controller return type accordingly and preserve use of IncidentResponse::fromEntity when converting entities to DTOs.
21-30: Return HTTP 201 for resource creation.POST endpoints that create resources should return
201 CREATEDrather than the default200 OK. Add@ResponseStatus(HttpStatus.CREATED)to the method.♻️ Proposed fix
+import org.springframework.http.HttpStatus;`@PostMapping` +@ResponseStatus(HttpStatus.CREATED) public IncidentResponse createIncident(`@RequestBody` IncidentRequest incidentRequest) {Note: The
createdByfield is already populated byIncidentService.createIncident(), which retrieves the authenticated user fromSecurityContextHolderand assigns it automatically. No validation constraints are currently defined onIncidentRequest, so@Validis not applicable at this time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/incident/controller/IncidentController.java` around lines 21 - 30, The createIncident POST handler currently returns 200 OK by default; update the IncidentController.createIncident method to annotate it with `@ResponseStatus`(HttpStatus.CREATED) so it returns HTTP 201 on successful creation, keeping the existing behavior of calling incidentService.createIncident(incident) and returning IncidentResponse.fromEntity(saved); no changes to IncidentRequest validation or createdBy population are required.
🤖 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/team6backend/incident/dto/IncidentRequest.java`:
- Around line 7-12: Add Bean Validation annotations to the IncidentRequest DTO:
mark subject with `@NotBlank` and incidentCategory with `@NotNull` (and description
optionally with `@Size` or `@NotBlank` if required) so null/empty values are
rejected before persistence; update the controller method createIncident in
IncidentController to accept `@Valid` `@RequestBody` IncidentRequest incidentRequest
so validation is triggered; ensure javax.validation (jakarta.validation)
annotations are imported and that the application has validation
starter/dependency enabled.
---
Nitpick comments:
In
`@src/main/java/org/example/team6backend/incident/controller/IncidentController.java`:
- Around line 32-38: Modify getAllIncidents to accept Spring Pageable (or
`@RequestParam` page/size and build one) and call
incidentService.findAll(pageable) instead of the parameterless findAll(); update
IncidentService.findAll(Pageable) to return Page<Incident>, then map the Page
content via IncidentResponse::fromEntity and return a paged response (e.g.,
Page<IncidentResponse> or ResponseEntity<Page<IncidentResponse>>). Ensure you
change the controller return type accordingly and preserve use of
IncidentResponse::fromEntity when converting entities to DTOs.
- Around line 21-30: The createIncident POST handler currently returns 200 OK by
default; update the IncidentController.createIncident method to annotate it with
`@ResponseStatus`(HttpStatus.CREATED) so it returns HTTP 201 on successful
creation, keeping the existing behavior of calling
incidentService.createIncident(incident) and returning
IncidentResponse.fromEntity(saved); no changes to IncidentRequest validation or
createdBy population are required.
In `@src/main/java/org/example/team6backend/incident/entity/Incident.java`:
- Around line 26-27: The Incident entity has a modifiedBy field but no
modifiedAt timestamp; add a new field (e.g., private Instant modifiedAt or
LocalDateTime modifiedAt) to the Incident class to record when modifications
occur, annotate it appropriately for persistence (e.g., `@Column` and/or
`@LastModifiedDate/`@Temporal as used elsewhere in the project), update
constructors/getters/setters and any builders or DTO mappers that touch
Incident, and ensure the service/repository layer that sets modifiedBy also sets
modifiedAt so the audit trail remains consistent with the modifiedBy field.
- Around line 23-30: Update the three `@ManyToOne` associations in the Incident
entity (fields createdBy, modifiedBy, assignedTo) to use lazy loading by adding
fetch = FetchType.LAZY to each annotation; ensure you import
javax.persistence.FetchType. Keep business logic that expects user data working
by explicitly fetching users when needed (e.g., use JOIN FETCH in repository
queries or map to DTOs) to avoid LazyInitializationException at runtime.
- Around line 8-9: The Incident entity currently defines manual getters/setters
while other entities (e.g., AppUser) use Lombok; replace the boilerplate by
adding Lombok annotations to the Incident class (for example `@Data` plus
`@NoArgsConstructor` and `@AllArgsConstructor` or `@Builder` as needed), add the
corresponding lombok imports, and remove the explicit getter/setter methods so
the class relies on Lombok-generated accessors; ensure the class-level
annotation is placed on class Incident and run a compile to confirm no remaining
manually defined accessors conflict.
In
`@src/main/java/org/example/team6backend/incident/repository/IncidentRepository.java`:
- Around line 11-12: Fix the minor formatting inconsistency in the repository
interface: in the method declaration for findByAssignedTo(AppUser user) (the
symbol to change is the return type on the line containing findByAssignedTo),
remove the unintended space between "List" and "<Incident>" so both method
signatures use the same "List<Incident>" formatting as findByCreatedBy(AppUser
user).
In
`@src/main/java/org/example/team6backend/incident/service/IncidentService.java`:
- Around line 24-27: The createIncident method currently casts
Authentication.getPrincipal() directly to CustomUserDetails which can fail if
the principal type changes; update the method to first retrieve Authentication
from SecurityContextHolder, check that auth.getPrincipal() is an instance of
CustomUserDetails (using instanceof), then cast and proceed to get AppUser; if
the principal is not CustomUserDetails, handle gracefully (throw a clear
unchecked exception or return a 401/forbidden result) so createIncident,
Authentication, SecurityContextHolder, CustomUserDetails and AppUser are guarded
against configuration changes.
- Around line 45-46: The method signature in IncidentService.findAll has a
formatting inconsistency: remove the extra space between the generic type and
the raw type so it reads List<Incident> findAll() instead of List <Incident>
findAll(); update the method declaration in the IncidentService class
accordingly (keeping return incidentRepository.findAll() 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae23c52e-f608-4a93-85a9-04767ed51d9a
📒 Files selected for processing (17)
src/main/java/org/example/team6backend/controller/AdminController.javasrc/main/java/org/example/team6backend/incident/controller/IncidentController.javasrc/main/java/org/example/team6backend/incident/dto/IncidentRequest.javasrc/main/java/org/example/team6backend/incident/dto/IncidentResponse.javasrc/main/java/org/example/team6backend/incident/entity/Incident.javasrc/main/java/org/example/team6backend/incident/entity/IncidentCategory.javasrc/main/java/org/example/team6backend/incident/entity/IncidentStatus.javasrc/main/java/org/example/team6backend/incident/repository/IncidentRepository.javasrc/main/java/org/example/team6backend/incident/service/IncidentService.javasrc/main/java/org/example/team6backend/security/CustomOAuth2UserService.javasrc/main/java/org/example/team6backend/security/CustomUserDetails.javasrc/main/java/org/example/team6backend/user/controller/UserController.javasrc/main/java/org/example/team6backend/user/dto/UserResponse.javasrc/main/java/org/example/team6backend/user/entity/AppUser.javasrc/main/java/org/example/team6backend/user/entity/UserRole.javasrc/main/java/org/example/team6backend/user/repository/AppUserRepository.javasrc/main/java/org/example/team6backend/user/service/UserService.java
# Conflicts: # src/main/java/org/example/team6backend/controller/AdminController.java # src/main/java/org/example/team6backend/user/repository/AppUserRepository.java # src/main/java/org/example/team6backend/user/service/UserService.java
…eorganize packages
Summary by CodeRabbit
New Features
Refactor