Refactor and pagination#34
Merged
Merged
Conversation
- Updated IncidentService to support pagination (Pageable) for:
- All incidents
- CreatedBy incidents (my incidents)
- AssignedTo incidents
- Updated IncidentController to handle paginated responses and map to IncidentResponse
- Added @PreAuthorize annotations for role-based access control on endpoints
- Updated repository methods to support Pageable
- Configured Flyway to handle baseline-on-migrate for existing database
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughReplaces an unpaginated incident list endpoint with three role-restricted, paginated endpoints and moves service/repository methods to pageable signatures. Adds createdAt/updatedAt timestamps, explicit column/join mappings on Incident, Flyway baseline config, and security-aware user resolution in service methods. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as IncidentController
participant Security as SecurityContext
participant Service as IncidentService
participant Repository as IncidentRepository
participant Database
Client->>Controller: GET /myincident?page=0&size=10
Controller->>Controller: `@PreAuthorize` checks RESIDENT role
Controller->>Service: findByCreatedBy(pageable)
Service->>Security: SecurityContextHolder.getContext()
Security-->>Service: CurrentUserDetails/AppUser
Service->>Repository: findByCreatedBy(user, pageable)
Repository->>Database: SELECT ... WHERE created_by_id = ? LIMIT/OFFSET
Database-->>Repository: paginated Incident rows
Repository-->>Service: Page<Incident>
Service-->>Controller: Page<Incident> (with default sort applied)
Controller->>Controller: map Page<Incident> -> Page<IncidentResponse>
Controller-->>Client: Page<IncidentResponse> + pagination metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 2
🧹 Nitpick comments (3)
src/main/java/org/example/team6backend/incident/repository/IncidentRepository.java (1)
13-14: Back these new paginated filters with indexes.
findByCreatedBy(...)andfindByAssignedTo(...)are now user-facing list paths.V3__create_incident_table.sqlcreatescreated_by_idandassigned_to_id, but no supporting indexes, so both the page query and the count query will degrade into table scans asincidentgrows. Please add indexes in a follow-up migration, ideally matching whatever default sort you settle on.🤖 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 13 - 14, Add supporting indexes for the new paginated filters so queries for findByCreatedBy(AppUser, Pageable) and findByAssignedTo(AppUser, Pageable) don't degenerate into full table scans; create a new migration (e.g., V4__add_incident_indexes.sql) that adds indexes on incident(created_by_id) and incident(assigned_to_id) and, if you choose a default sort (e.g., created_at DESC), add composite indexes matching that sort such as incident(created_by_id, created_at) and incident(assigned_to_id, created_at) to optimize both the pageable content query and the count query.src/main/resources/application.yml (1)
20-23: Verify this Flyway baseline strategy before enabling it globally.
baseline-on-migrateis only safe if the baseline version matches the schema that's already live. With versioned migrations already checked in, this flag alone can make the first Flyway run against a non-empty database ambiguous. Please set the matching baseline version or isolate this to the one-time bootstrap environment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/application.yml` around lines 20 - 23, The Flyway config enabling baseline-on-migrate is risky; either set an explicit matching baseline version with the Flyway property baseline-version to the live schema version (so baseline-on-migrate: true is safe), or remove/disable baseline-on-migrate from global configuration and move it into a one-time bootstrap profile (e.g., an application-bootstrap profile or environment-specific config) so only the bootstrap environment uses baseline-on-migrate; update the configuration keys flyway.baseline-on-migrate and flyway.baseline-version (or place them under a dedicated Spring profile) accordingly.src/main/java/org/example/team6backend/incident/controller/IncidentController.java (1)
24-26: ThePOST /api/incidentsendpoint is already protected via URL-level authentication.
SecurityConfig.securityFilterChain()specifies.anyRequest().authenticated()(line 36), which requires authentication for all requests except those explicitly permitted. Since/api/incidentsis not in the permitAll list, the endpoint is protected and will return 401/403 before reaching the service layer.However, adding
@PreAuthorize("hasRole('RESIDENT')")tocreateIncident()would improve consistency with the three GET endpoints in this controller and make the access control contract explicit rather than implicit.🤖 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 24 - 26, The createIncident endpoint in IncidentController (method createIncident) lacks the explicit role-based annotation; add the `@PreAuthorize`("hasRole('RESIDENT')") annotation to the createIncident(`@Valid` `@RequestBody` IncidentRequest incidentRequest) method so access is explicitly restricted to users with the RESIDENT role (matching the existing GET endpoints), and ensure the class imports and method-level security is enabled so the annotation is honored.
🤖 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/entity/Incident.java`:
- Around line 37-41: The Incident entity's timestamps are not maintained
automatically; add JPA lifecycle callbacks to set createdAt and updatedAt
consistently: implement a `@PrePersist` method (e.g., prePersist or
setTimestampsOnCreate) that sets createdAt and updatedAt to LocalDateTime.now()
when the entity is first persisted, and a `@PreUpdate` method (e.g., preUpdate or
setUpdatedTimestamp) that updates updatedAt to LocalDateTime.now() on every
update; follow the same pattern used in AppUser.java and place these methods
inside the Incident class so any persist/update path will maintain the createdAt
and updatedAt fields.
In
`@src/main/java/org/example/team6backend/incident/service/IncidentService.java`:
- Around line 39-57: IncidentService's pagination methods (findAll,
findByCreatedBy, findByAssignedTo) pass the incoming Pageable straight to the
repository, which can produce unstable page boundaries when no sort is provided;
add a private helper (e.g., ensureDefaultSort(Pageable)) in IncidentService that
checks pageable.getSort().isUnsorted() and, if unsorted, returns a new
PageRequest preserving page number and size but adding a stable default Sort
(for example Sort.by("id").ascending() or Sort.by("createdAt").descending());
call this helper from each of the three methods before delegating to
incidentRepository.findAll/findByCreatedBy/findByAssignedTo so every query uses
a deterministic sort when the client omits one.
---
Nitpick comments:
In
`@src/main/java/org/example/team6backend/incident/controller/IncidentController.java`:
- Around line 24-26: The createIncident endpoint in IncidentController (method
createIncident) lacks the explicit role-based annotation; add the
`@PreAuthorize`("hasRole('RESIDENT')") annotation to the createIncident(`@Valid`
`@RequestBody` IncidentRequest incidentRequest) method so access is explicitly
restricted to users with the RESIDENT role (matching the existing GET
endpoints), and ensure the class imports and method-level security is enabled so
the annotation is honored.
In
`@src/main/java/org/example/team6backend/incident/repository/IncidentRepository.java`:
- Around line 13-14: Add supporting indexes for the new paginated filters so
queries for findByCreatedBy(AppUser, Pageable) and findByAssignedTo(AppUser,
Pageable) don't degenerate into full table scans; create a new migration (e.g.,
V4__add_incident_indexes.sql) that adds indexes on incident(created_by_id) and
incident(assigned_to_id) and, if you choose a default sort (e.g., created_at
DESC), add composite indexes matching that sort such as incident(created_by_id,
created_at) and incident(assigned_to_id, created_at) to optimize both the
pageable content query and the count query.
In `@src/main/resources/application.yml`:
- Around line 20-23: The Flyway config enabling baseline-on-migrate is risky;
either set an explicit matching baseline version with the Flyway property
baseline-version to the live schema version (so baseline-on-migrate: true is
safe), or remove/disable baseline-on-migrate from global configuration and move
it into a one-time bootstrap profile (e.g., an application-bootstrap profile or
environment-specific config) so only the bootstrap environment uses
baseline-on-migrate; update the configuration keys flyway.baseline-on-migrate
and flyway.baseline-version (or place them under a dedicated Spring profile)
accordingly.
🪄 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: d7b77b6d-d16b-4fdf-b1cc-8a3ae443bed8
📒 Files selected for processing (5)
src/main/java/org/example/team6backend/incident/controller/IncidentController.javasrc/main/java/org/example/team6backend/incident/entity/Incident.javasrc/main/java/org/example/team6backend/incident/repository/IncidentRepository.javasrc/main/java/org/example/team6backend/incident/service/IncidentService.javasrc/main/resources/application.yml
This was referenced Apr 2, 2026
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.
updatedAtfield to Incident entitySummary by CodeRabbit
New Features
Improvements