Refactor/comment controller rest#97
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe pull request converts Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 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 (1)
src/test/java/org/example/team6backend/comment/controller/CommentControllerTest.java (1)
60-67: Rename the test to match the REST behavior.The test no longer verifies a redirect, so the current name is stale.
♻️ Proposed rename
- void shouldCreateCommentAndRedirectToIncidentPage() throws Exception { + void shouldCreateCommentWithJsonPayload() throws Exception {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/team6backend/comment/controller/CommentControllerTest.java` around lines 60 - 67, The test method shouldCreateCommentAndRedirectToIncidentPage() has a stale name because it no longer asserts a redirect; rename it to reflect the actual behavior (e.g., shouldCreateCommentAndReturnOk or shouldCreateCommentAndRespondWithStatusOk) and update the method name in CommentControllerTest accordingly so any references (the `@Test` method name shouldCreateCommentAndRedirectToIncidentPage) match the new name; do not change the assertions or request payload, only rename the test method for clarity.
🤖 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/comment/controller/CommentController.java`:
- Around line 31-32: The createComment controller method currently uses
request.getUserId(), allowing client-controlled impersonation; update the method
signature to accept `@AuthenticationPrincipal` CustomUserDetails userDetails and
pass userDetails.getUser().getId() into commentService.createComment instead of
request.getUserId(), and remove the userId field and its getter from
CommentRequest so the controller derives the user id from the authenticated
principal; keep the rest of the call (incidentId and message) unchanged and
mirror the pattern used in UserController/NotificationController.
In `@src/main/resources/templates/viewincident.html`:
- Around line 162-163: The viewIncident handler does not add the CSRF token to
the model while the template expects ${_csrf.token} and ${_csrf.headerName};
update the viewIncident method to supply the CSRF token the same way
createIncident does — accept a CsrfToken parameter (or obtain it from the
request) and call model.addAttribute("_csrf", csrf) so the template can read
token and headerName for the fetch POST to /comments.
---
Nitpick comments:
In
`@src/test/java/org/example/team6backend/comment/controller/CommentControllerTest.java`:
- Around line 60-67: The test method
shouldCreateCommentAndRedirectToIncidentPage() has a stale name because it no
longer asserts a redirect; rename it to reflect the actual behavior (e.g.,
shouldCreateCommentAndReturnOk or shouldCreateCommentAndRespondWithStatusOk) and
update the method name in CommentControllerTest accordingly so any references
(the `@Test` method name shouldCreateCommentAndRedirectToIncidentPage) match the
new name; do not change the assertions or request payload, only rename the test
method for clarity.
🪄 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: 11a5c1c0-9970-4c8e-a1f4-9c9e9de84ffb
📒 Files selected for processing (3)
src/main/java/org/example/team6backend/comment/controller/CommentController.javasrc/main/resources/templates/viewincident.htmlsrc/test/java/org/example/team6backend/comment/controller/CommentControllerTest.java
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/resources/static/viewincident.html (1)
159-172: Minor: textarea should have aname/id-based label and wire CSRF header name safely.Nitpicks on the new form:
- The
<textarea>has no associated<label>— accessibility-only concern, low priority.[csrfHeader]: csrfTokenuses the hidden-input value as a dynamic header name. IfcsrfHeaderrenders empty (see the CSRF model concern), this becomes header"": "", which some browsers silently drop and Spring will then reject. Consider defaulting to"X-CSRF-TOKEN"when the rendered value is empty, or validating before sending:if (!csrfHeader || !csrfToken) { console.error("CSRF token not available"); alert("Session expired, please reload the page."); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/static/viewincident.html` around lines 159 - 172, The textarea in the comment form (id="messageInput") lacks an associated label—add a <label> tied to messageInput (using for/id or a hidden label for visual design) to improve accessibility; and when building the CSRF header for the request, validate the hidden inputs csrfHeader and csrfToken (ids "csrfHeader" and "csrfToken") and default the header name to "X-CSRF-TOKEN" if csrfHeader is empty or missing, or show a user-friendly error/abort (e.g., log and alert) when either value is absent before sending the request to avoid sending an empty header name.
🤖 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/resources/static/viewincident.html`:
- Around line 377-415: In submitComment, prevent double-submits by disabling the
form submitter during the async fetch and re-enabling it in finally: capture the
button via event.submitter (or fallback to
document.getElementById("submitCommentBtn") if you use an explicit id), set
disabled = true before the fetch and ensure disabled = false in a finally block
after loadComments/loadActivityLog; also add a client-side guard that reads
document.getElementById("userIdInput").value.trim() and alerts/returns if empty
(so you don't send a request with blank userId), keeping the existing behavior
of trimming message and existing headers (csrfToken/csrfHeader) and reusing the
same JSON payload structure.
---
Nitpick comments:
In `@src/main/resources/static/viewincident.html`:
- Around line 159-172: The textarea in the comment form (id="messageInput")
lacks an associated label—add a <label> tied to messageInput (using for/id or a
hidden label for visual design) to improve accessibility; and when building the
CSRF header for the request, validate the hidden inputs csrfHeader and csrfToken
(ids "csrfHeader" and "csrfToken") and default the header name to "X-CSRF-TOKEN"
if csrfHeader is empty or missing, or show a user-friendly error/abort (e.g.,
log and alert) when either value is absent before sending the request to avoid
sending an empty header name.
🪄 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: c8a6b645-9cf6-482f-8b84-c359c54418e0
📒 Files selected for processing (1)
src/main/resources/static/viewincident.html
This was referenced Apr 23, 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
Release Notes