Refactor incident creation flow and dashboard links#70
Merged
Conversation
- createincident.html: Added th:action to form, removed entire script section.
- dashboard.html: Updated "Create new incident" link to /create-incident.
- IncidentController.java: Added @AuthenticationPrincipal CustomUserDetails to controller methods.
- IncidentService.java: Replaced unused userDetails with AppUser user in method parameters.
- PageController.java:
- Injected IncidentService.
- Added switch-case on user.getRole() to fetch incidents based on role.
- Added CsrfToken to model for forms.
- Added POST endpoint for submitting new incidents.
- Added GET endpoint to view a created incident.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 33 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughControllers and templates now use the authenticated user for incident queries; incident creation moved from client-side fetch to server-side Thymeleaf form handling. Service methods were changed to accept explicit Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PageController
participant IncidentService
participant IncidentRepository
participant Database
Note over User,Database: Incident creation flow
User->>PageController: POST /create-incident (form)
PageController->>PageController: extract CustomUserDetails → AppUser
PageController->>IncidentService: createIncident(mapped Incident with createdBy)
IncidentService->>IncidentRepository: save(incident)
IncidentRepository->>Database: INSERT incident
Database-->>IncidentRepository: persisted incident
IncidentRepository-->>IncidentService: persisted incident
IncidentService-->>PageController: persisted incident
PageController-->>User: Redirect /incident/{id}
Note over User,Database: Dashboard load flow
User->>PageController: GET /dashboard (pageable)
PageController->>PageController: extract user & role
alt RESIDENT
PageController->>IncidentService: findByCreatedBy(user, pageable)
else HANDLER
PageController->>IncidentService: findByAssignedTo(user, pageable)
else ADMIN
PageController->>IncidentService: findAll(pageable)
else
PageController->>PageController: set empty incidents
end
IncidentService->>IncidentRepository: query (paged)
IncidentRepository->>Database: SELECT ...
Database-->>IncidentRepository: paged results
IncidentRepository-->>IncidentService: Page<Incident>
IncidentService-->>PageController: Page<Incident>
PageController-->>User: Render dashboard.html
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 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: 3
🧹 Nitpick comments (1)
src/main/java/org/example/team6backend/page/PageController.java (1)
42-50: This dashboard query is currently dead work.
src/main/resources/templates/dashboard.htmlstill loads incidents vialoadIncidents()and never reads theincidentsmodel attribute, so this adds an extra DB read on every dashboard request without changing the page. Either renderincidentsserver-side or drop the controller fetch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/page/PageController.java` around lines 42 - 50, The controller currently performs an extra DB read by populating Page<Incident> incidents in PageController (switch on user.getRole()) and adding it to the model, but templates/dashboard.html ignores that model and uses loadIncidents() client-side; either remove the server-side fetch or make the template consume it. Fix options: 1) Remove the switch + incidentService calls and the model.addAttribute("incidents", incidents) to avoid the redundant DB query (delete code referencing incidents in PageController), or 2) Change the template and/or JavaScript so dashboard.html uses the server-provided incidents (stop calling loadIncidents() and render incidents server-side), ensuring PageController’s incidents model is the single source of truth; update or remove loadIncidents() accordingly.
🤖 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/page/PageController.java`:
- Around line 101-109: The viewIncident handler (viewIncident) currently renders
any incident by id via incidentService.findById without authorizing the current
user; update viewIncident to fetch the authenticated principal (via
SecurityContextHolder or injected Principal) and enforce access control (e.g.,
allow if user is the incident creator, an assignee, or has an admin role) before
adding the incident to the model; if the user is not authorized return a 403 or
redirect to /dashboard. You can implement the check inline in viewIncident or
add a helper like incidentService.canView(Incident, String principalName) and
call that, and ensure SecurityConfig remains consistent with this
controller-side authorization.
- Line 82: The POST handler for "/create-incident" in PageController lacks an
authorization check, allowing unauthorized roles to submit incidents; restrict
it by adding a method-level role check (e.g., annotate the controller method
handling POST "/create-incident" with
`@PreAuthorize`("hasAnyRole('RESIDENT','ADMIN')") or
`@Secured`({"ROLE_RESIDENT","ROLE_ADMIN"}) depending on your project convention)
and import the corresponding annotation; ensure method-level security is enabled
if needed so only RESIDENT/ADMIN can call the POST.
- Around line 83-84: The submitIncident handler currently binds IncidentRequest
without validation; update the method signature for submitIncident to annotate
the IncidentRequest parameter with `@Valid` and add a BindingResult parameter
immediately after it (e.g., BindingResult bindingResult), then in the method
check bindingResult.hasErrors() and if true re-populate any needed model
attributes and return the form view to re-render validation errors instead of
proceeding to persist; reference the submitIncident method, the IncidentRequest
DTO, and BindingResult to locate and implement this change.
---
Nitpick comments:
In `@src/main/java/org/example/team6backend/page/PageController.java`:
- Around line 42-50: The controller currently performs an extra DB read by
populating Page<Incident> incidents in PageController (switch on user.getRole())
and adding it to the model, but templates/dashboard.html ignores that model and
uses loadIncidents() client-side; either remove the server-side fetch or make
the template consume it. Fix options: 1) Remove the switch + incidentService
calls and the model.addAttribute("incidents", incidents) to avoid the redundant
DB query (delete code referencing incidents in PageController), or 2) Change the
template and/or JavaScript so dashboard.html uses the server-provided incidents
(stop calling loadIncidents() and render incidents server-side), ensuring
PageController’s incidents model is the single source of truth; update or remove
loadIncidents() 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: b10e4956-d52d-4567-9c9b-eda7e47ce7bd
📒 Files selected for processing (5)
src/main/java/org/example/team6backend/incident/controller/IncidentController.javasrc/main/java/org/example/team6backend/incident/service/IncidentService.javasrc/main/java/org/example/team6backend/page/PageController.javasrc/main/resources/templates/createincident.htmlsrc/main/resources/templates/dashboard.html
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/main/java/org/example/team6backend/page/PageController.java (2)
100-109:⚠️ Potential issue | 🔴 CriticalMissing authorization check for incident viewing.
Any authenticated user can view any incident by ID. Verify the current user is the creator, assignee, or an admin before rendering.
🔒 Suggested fix
`@GetMapping`("/incident/{id}") - public String viewIncident(`@PathVariable` Long id, Model model) { + public String viewIncident(`@PathVariable` Long id, + `@AuthenticationPrincipal` CustomUserDetails userDetails, Model model) { Incident incident = incidentService.findById(id); if (incident == null) { return "redirect:/dashboard"; } + + AppUser currentUser = userDetails.getUser(); + boolean isCreator = incident.getCreatedBy() != null && + incident.getCreatedBy().getId().equals(currentUser.getId()); + boolean isAssignee = incident.getAssignedTo() != null && + incident.getAssignedTo().getId().equals(currentUser.getId()); + boolean isAdmin = "ADMIN".equals(currentUser.getRole().name()); + + if (!isCreator && !isAssignee && !isAdmin) { + return "redirect:/dashboard"; + } + model.addAttribute("incident", incident); return "view-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/page/PageController.java` around lines 100 - 109, The viewIncident handler currently returns any incident to any authenticated user; modify the method so after retrieving Incident via incidentService.findById(id) you obtain the current user (e.g., from SecurityContextHolder or an injected UserService) and verify that the current user's id or username equals incident.getCreator() or incident.getAssignee(), or that the user has an admin authority (e.g., hasRole("ADMIN")); if the check fails, return a redirect (e.g., "redirect:/dashboard" or an access-denied page) instead of rendering "view-incident". Ensure the authorization check is performed before adding the incident to the Model and before returning the view.
81-98:⚠️ Potential issue | 🟠 MajorMultiple issues in the POST handler.
Authorization gap: The GET handler restricts to
RESIDENT/ADMIN, but this POST lacks the same check. Add@PreAuthorize("hasAnyRole('RESIDENT', 'ADMIN')")or an equivalent guard.Missing validation:
IncidentRequesthas@NotBlank/@NotNullconstraints but@Validis not applied, allowing bypassed validation.Redundant
setCreatedBy: Line 91 setscreatedBy, butIncidentService.createIncident(see context snippet 1 at lines 36-47) overwrites it fromSecurityContextHolder. Remove line 91 to avoid confusion.Model attributes lost on redirect: Lines 95-96 add attributes to
model, but these are discarded during redirect. UseRedirectAttributes.addFlashAttribute()if you need to pass data, or remove these lines.🔧 Suggested fixes
+ `@PreAuthorize`("hasAnyRole('RESIDENT', 'ADMIN')") `@PostMapping`("/create-incident") public String submitIncident(`@AuthenticationPrincipal` CustomUserDetails userDetails, - `@ModelAttribute` IncidentRequest incidentRequest, Model model) { + `@Valid` `@ModelAttribute` IncidentRequest incidentRequest, + BindingResult bindingResult, Model model, RedirectAttributes redirectAttributes) { AppUser user = userDetails.getUser(); - String role = user.getRole().name(); + + if (bindingResult.hasErrors()) { + model.addAttribute("role", user.getRole().name()); + model.addAttribute("user", user); + return "createincident"; + } Incident incident = new Incident(); incident.setSubject(incidentRequest.getSubject()); incident.setDescription(incidentRequest.getDescription()); incident.setIncidentCategory(incidentRequest.getIncidentCategory()); - incident.setCreatedBy(user); Incident saved = incidentService.createIncident(incident); - model.addAttribute("success", "Incident created successfully!"); - model.addAttribute("incidentRequest", incidentRequest); + redirectAttributes.addFlashAttribute("success", "Incident created successfully!"); return "redirect:/incident/" + saved.getId(); }Add imports:
import org.springframework.validation.BindingResult; import org.springframework.web.servlet.mvc.support.RedirectAttributes; import jakarta.validation.Valid; import org.springframework.security.access.prepost.PreAuthorize;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/page/PageController.java` around lines 81 - 98, The POST handler submitIncident lacks authorization, validation, redundantly sets createdBy, and misuses Model on redirect: annotate submitIncident with `@PreAuthorize`("hasAnyRole('RESIDENT','ADMIN')"); add `@Valid` to the IncidentRequest parameter and a BindingResult parameter to validate input before proceeding; remove the redundant incident.setCreatedBy(...) assignment (createIncident sets it from SecurityContextHolder); and replace the model.addAttribute(...) calls with a RedirectAttributes parameter and redirectAttributes.addFlashAttribute(...) (or drop them if not needed); also add the suggested imports (BindingResult, RedirectAttributes, `@Valid`, PreAuthorize).
🧹 Nitpick comments (2)
src/main/java/org/example/team6backend/incident/service/IncidentService.java (1)
64-66: Consider returningOptional<Incident>instead of nullable.Returning
nullfor missing entities is functional, but returningOptional<Incident>is more idiomatic with Spring Data and makes null-handling explicit at the call site, reducing the risk of NPE if callers forget to check.♻️ Suggested refactor
- public Incident findById(Long id) { - return incidentRepository.findById(id).orElse(null); + public Optional<Incident> findById(Long id) { + return incidentRepository.findById(id); }Add import:
import java.util.Optional;Then in
PageController.viewIncident:- Incident incident = incidentService.findById(id); - if (incident == null) { + Optional<Incident> optIncident = incidentService.findById(id); + if (optIncident.isEmpty()) { return "redirect:/dashboard"; } + Incident incident = optIncident.get();🤖 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 64 - 66, Change IncidentService.findById to return Optional<Incident> instead of nullable: keep using incidentRepository.findById(id) but return it directly as Optional; update all callers (e.g., PageController.viewIncident) to handle the Optional (use isPresent/get or map/orElseThrow as appropriate). Add import java.util.Optional if missing and remove any null-checking logic that assumed a null return. Ensure method signature and usages are updated consistently to avoid compilation errors.src/main/java/org/example/team6backend/page/PageController.java (1)
34-55: Role-based incident loading looks correct.The switch expression cleanly handles role-based queries. Minor nit: line 38 uses
userDetails.getUser()instead of the already-assigneduservariable from line 37.✨ Minor consistency fix
AppUser user = userDetails.getUser(); - model.addAttribute("user", userDetails.getUser()); + model.addAttribute("user", 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/page/PageController.java` around lines 34 - 55, In the dashboard method, replace the redundant userDetails.getUser() usage with the already-assigned local variable user (set from userDetails.getUser()) when calling model.addAttribute("user", ...); update the model.addAttribute call so it uses user to keep consistency with the earlier assignment and avoid the duplicate getter call in the method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main/java/org/example/team6backend/page/PageController.java`:
- Around line 100-109: The viewIncident handler currently returns any incident
to any authenticated user; modify the method so after retrieving Incident via
incidentService.findById(id) you obtain the current user (e.g., from
SecurityContextHolder or an injected UserService) and verify that the current
user's id or username equals incident.getCreator() or incident.getAssignee(), or
that the user has an admin authority (e.g., hasRole("ADMIN")); if the check
fails, return a redirect (e.g., "redirect:/dashboard" or an access-denied page)
instead of rendering "view-incident". Ensure the authorization check is
performed before adding the incident to the Model and before returning the view.
- Around line 81-98: The POST handler submitIncident lacks authorization,
validation, redundantly sets createdBy, and misuses Model on redirect: annotate
submitIncident with `@PreAuthorize`("hasAnyRole('RESIDENT','ADMIN')"); add `@Valid`
to the IncidentRequest parameter and a BindingResult parameter to validate input
before proceeding; remove the redundant incident.setCreatedBy(...) assignment
(createIncident sets it from SecurityContextHolder); and replace the
model.addAttribute(...) calls with a RedirectAttributes parameter and
redirectAttributes.addFlashAttribute(...) (or drop them if not needed); also add
the suggested imports (BindingResult, RedirectAttributes, `@Valid`, PreAuthorize).
---
Nitpick comments:
In
`@src/main/java/org/example/team6backend/incident/service/IncidentService.java`:
- Around line 64-66: Change IncidentService.findById to return
Optional<Incident> instead of nullable: keep using
incidentRepository.findById(id) but return it directly as Optional; update all
callers (e.g., PageController.viewIncident) to handle the Optional (use
isPresent/get or map/orElseThrow as appropriate). Add import java.util.Optional
if missing and remove any null-checking logic that assumed a null return. Ensure
method signature and usages are updated consistently to avoid compilation
errors.
In `@src/main/java/org/example/team6backend/page/PageController.java`:
- Around line 34-55: In the dashboard method, replace the redundant
userDetails.getUser() usage with the already-assigned local variable user (set
from userDetails.getUser()) when calling model.addAttribute("user", ...); update
the model.addAttribute call so it uses user to keep consistency with the earlier
assignment and avoid the duplicate getter call in the method.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8449c36c-fd20-4ee2-b1f0-11167584a34b
📒 Files selected for processing (3)
src/main/java/org/example/team6backend/incident/controller/IncidentController.javasrc/main/java/org/example/team6backend/incident/service/IncidentService.javasrc/main/java/org/example/team6backend/page/PageController.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/example/team6backend/incident/controller/IncidentController.java
- restrict access to ADMIN, assigned HANDLER, or owning RESIDENT - return 403 Forbidden for unauthorized access - fix role comparison using enum instead of string
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/java/org/example/team6backend/page/PageController.java (2)
124-124: Prefer enum comparison for type safety.Similar to the suggestion for the GET endpoint, using enum comparison would be more robust than string comparison.
♻️ Suggested improvement
- boolean isAdmin = user.getRole().name().equals("ADMIN"); + boolean isAdmin = user.getRole() == Role.ADMIN;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/page/PageController.java` at line 124, Replace the string comparison of the user's role with a direct enum comparison: change the boolean assignment that currently uses user.getRole().name().equals("ADMIN") to compare the enum returned by getRole() against the ADMIN constant (e.g., user.getRole() == Role.ADMIN or the actual enum type used), updating the declaration of isAdmin in PageController to use that enum comparison for type safety.
77-77: Consider comparing roles using enum values for type safety.Using string comparison is functional but comparing enum values directly would be more type-safe and consistent with the switch expression used in
dashboard().♻️ Suggested improvement
- if (role.equals("RESIDENT") || role.equals("ADMIN")) { + if (user.getRole() == Role.RESIDENT || user.getRole() == Role.ADMIN) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/page/PageController.java` at line 77, The code currently compares a String role via role.equals("RESIDENT") || role.equals("ADMIN") in PageController; change this to use the role enum used in the dashboard() switch expression (e.g., Role or UserRole) for type safety: convert the incoming role String to the enum with Role.valueOf(role) (handle IllegalArgumentException/null safely) or change the controller to accept the enum directly, then replace the String comparisons with enum comparisons like roleEnum == Role.RESIDENT || roleEnum == Role.ADMIN (or use EnumSet.of(Role.RESIDENT, Role.ADMIN).contains(roleEnum)) and update any method signatures or error handling accordingly to ensure consistent enum-based checks throughout PageController.
🤖 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/page/PageController.java`:
- Line 128: The current assignment to isResident calls
incident.getCreatedBy().getId() and can NPE if createdBy is null; update the
logic in PageController where isResident is set so you first check createdBy is
non-null and then compare IDs (e.g., guard with incident.getCreatedBy() != null
&& Objects.equals(incident.getCreatedBy().getId(), user.getId())), or otherwise
use a null-safe comparator to compare createdBy.getId() and user.getId(); ensure
you import java.util.Objects if using Objects.equals.
- Around line 107-109: The controller is adding model attributes before
returning a redirect so they are lost; update the handler in PageController (the
method that currently calls model.addAttribute("success", ...) and
model.addAttribute("incidentRequest", ...)) to accept a RedirectAttributes
parameter and replace those model.addAttribute calls with
redirectAttributes.addFlashAttribute("success", ...) and
redirectAttributes.addFlashAttribute("incidentRequest", ...), then keep the
existing "redirect:/incident/" + saved.getId() return so the flash attributes
survive the redirect.
- Around line 93-97: The form re-render on validation error in the
createIncident POST branch (when bindingResult.hasErrors()) is missing the CSRF
token in the model, causing SpEL failures in createincident template; update the
POST handler in PageController to accept HttpServletRequest (like the GET
handler does), retrieve the CsrfToken from the request
(javax.servlet.http.HttpServletRequest ->
request.getAttribute(CsrfToken.class.getName()) or using
org.springframework.security.web.csrf.CsrfToken), and add the token attributes
to the model (the same keys your template expects, e.g., "_csrf.parameterName"
and "_csrf.token") before returning "createincident" so the template can render
without error.
---
Nitpick comments:
In `@src/main/java/org/example/team6backend/page/PageController.java`:
- Line 124: Replace the string comparison of the user's role with a direct enum
comparison: change the boolean assignment that currently uses
user.getRole().name().equals("ADMIN") to compare the enum returned by getRole()
against the ADMIN constant (e.g., user.getRole() == Role.ADMIN or the actual
enum type used), updating the declaration of isAdmin in PageController to use
that enum comparison for type safety.
- Line 77: The code currently compares a String role via role.equals("RESIDENT")
|| role.equals("ADMIN") in PageController; change this to use the role enum used
in the dashboard() switch expression (e.g., Role or UserRole) for type safety:
convert the incoming role String to the enum with Role.valueOf(role) (handle
IllegalArgumentException/null safely) or change the controller to accept the
enum directly, then replace the String comparisons with enum comparisons like
roleEnum == Role.RESIDENT || roleEnum == Role.ADMIN (or use
EnumSet.of(Role.RESIDENT, Role.ADMIN).contains(roleEnum)) and update any method
signatures or error handling accordingly to ensure consistent enum-based checks
throughout PageController.
🪄 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: bdf2e6b2-3fdc-4fb3-9f81-15bfe1778d16
📒 Files selected for processing (2)
src/main/java/org/example/team6backend/page/PageController.javasrc/main/resources/templates/createincident.html
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/resources/templates/createincident.html
Comment on lines
+107
to
+109
| model.addAttribute("success", "Incident created successfully!"); | ||
| model.addAttribute("incidentRequest", incidentRequest); | ||
| return "redirect:/incident/" + saved.getId(); |
There was a problem hiding this comment.
Model attributes won't survive the redirect.
Lines 107-108 set success and incidentRequest as model attributes, but these are lost when performing an HTTP redirect. Use RedirectAttributes to flash messages across redirects.
🔧 Proposed fix
`@PostMapping`("/create-incident")
public String submitIncident(`@AuthenticationPrincipal` CustomUserDetails userDetails,
- `@Valid` `@ModelAttribute` IncidentRequest incidentRequest, BindingResult bindingResult, Model model) {
+ `@Valid` `@ModelAttribute` IncidentRequest incidentRequest, BindingResult bindingResult,
+ RedirectAttributes redirectAttributes, Model model) {
AppUser user = userDetails.getUser();
String role = user.getRole().name();
if (bindingResult.hasErrors()) {
model.addAttribute("role", role);
model.addAttribute("user", user);
return "createincident";
}
Incident incident = new Incident();
incident.setSubject(incidentRequest.getSubject());
incident.setDescription(incidentRequest.getDescription());
incident.setIncidentCategory(incidentRequest.getIncidentCategory());
incident.setCreatedBy(user);
Incident saved = incidentService.createIncident(incident);
- model.addAttribute("success", "Incident created successfully!");
- model.addAttribute("incidentRequest", incidentRequest);
+ redirectAttributes.addFlashAttribute("success", "Incident created successfully!");
return "redirect:/incident/" + saved.getId();
}📝 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
| model.addAttribute("success", "Incident created successfully!"); | |
| model.addAttribute("incidentRequest", incidentRequest); | |
| return "redirect:/incident/" + saved.getId(); | |
| `@PostMapping`("/create-incident") | |
| public String submitIncident(`@AuthenticationPrincipal` CustomUserDetails userDetails, | |
| `@Valid` `@ModelAttribute` IncidentRequest incidentRequest, BindingResult bindingResult, | |
| RedirectAttributes redirectAttributes, Model model) { | |
| AppUser user = userDetails.getUser(); | |
| String role = user.getRole().name(); | |
| if (bindingResult.hasErrors()) { | |
| model.addAttribute("role", role); | |
| model.addAttribute("user", user); | |
| return "createincident"; | |
| } | |
| Incident incident = new Incident(); | |
| incident.setSubject(incidentRequest.getSubject()); | |
| incident.setDescription(incidentRequest.getDescription()); | |
| incident.setIncidentCategory(incidentRequest.getIncidentCategory()); | |
| incident.setCreatedBy(user); | |
| Incident saved = incidentService.createIncident(incident); | |
| redirectAttributes.addFlashAttribute("success", "Incident created successfully!"); | |
| return "redirect:/incident/" + saved.getId(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/example/team6backend/page/PageController.java` around lines
107 - 109, The controller is adding model attributes before returning a redirect
so they are lost; update the handler in PageController (the method that
currently calls model.addAttribute("success", ...) and
model.addAttribute("incidentRequest", ...)) to accept a RedirectAttributes
parameter and replace those model.addAttribute calls with
redirectAttributes.addFlashAttribute("success", ...) and
redirectAttributes.addFlashAttribute("incidentRequest", ...), then keep the
existing "redirect:/incident/" + saved.getId() return so the flash attributes
survive the redirect.
|
|
||
| boolean isHandler = incident.getAssignedTo() != null && incident.getAssignedTo().getId().equals(user.getId()); | ||
|
|
||
| boolean isResident = incident.getCreatedBy().getId().equals(user.getId()); |
There was a problem hiding this comment.
Add null check for createdBy defensively.
While IncidentService.createIncident always sets createdBy, incidents created before this change or via other paths could have null createdBy, causing an NPE.
🛡️ Proposed fix
- boolean isResident = incident.getCreatedBy().getId().equals(user.getId());
+ boolean isResident = incident.getCreatedBy() != null && incident.getCreatedBy().getId().equals(user.getId());📝 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
| boolean isResident = incident.getCreatedBy().getId().equals(user.getId()); | |
| boolean isResident = incident.getCreatedBy() != null && incident.getCreatedBy().getId().equals(user.getId()); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/example/team6backend/page/PageController.java` at line 128,
The current assignment to isResident calls incident.getCreatedBy().getId() and
can NPE if createdBy is null; update the logic in PageController where
isResident is set so you first check createdBy is non-null and then compare IDs
(e.g., guard with incident.getCreatedBy() != null &&
Objects.equals(incident.getCreatedBy().getId(), user.getId())), or otherwise use
a null-safe comparator to compare createdBy.getId() and user.getId(); ensure you
import java.util.Objects if using Objects.equals.
Merged
This was referenced Apr 9, 2026
Merged
This was referenced Apr 22, 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.
Summary by CodeRabbit
New Features
Bug Fixes