fix: hide delete buttons unless signed on#2361
Conversation
|
No actionable comments were generated in the recent review. 🎉 WalkthroughUpdated project version and modified several content edit JSPs so the Delete link is rendered only when a contributor is present; no other behavioral or public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/webapp/WEB-INF/jsp/content/number/edit.jsp (1)
113-114:⚠️ Potential issue | 🟠 MajorGuard against null contributor before comparing IDs to prevent NPE.
Lines 113-114 access
contributor.idwithout checking ifcontributoris null. When unauthenticated visitors access this page,contributorwill be null from the session, throwing aNullPointerException. The same issue exists inword/edit.jspat line 173.Add a null guard before the ID comparison:
Suggested fix
- <c:if test="${(not empty numberContributionEvents) - && (numberContributionEvents[0].contributor.id != contributor.id)}"> + <c:if test="${(not empty numberContributionEvents) and (not empty contributor) + && (numberContributionEvents[0].contributor.id != contributor.id)}">Apply the same guard to
word/edit.jspat line 173.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/WEB-INF/jsp/content/number/edit.jsp` around lines 113 - 114, The comparison on numberContributionEvents[0].contributor.id uses contributor.id without guarding for a null contributor, causing an NPE for unauthenticated users; update the conditional in edit.jsp to check contributor != null before comparing IDs (i.e., ensure contributor is not null && numberContributionEvents[0].contributor.id != contributor.id), and make the identical change at the equivalent check in word/edit.jsp (the conditional around line 173) so both pages safely handle null contributor sessions.
🤖 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/webapp/WEB-INF/jsp/content/word/edit.jsp`:
- Around line 163-165: The <c:if> guard is malformed: the opening tag is
immediately closed leaving the Delete anchor (<a
href="/content/word/delete/${word.id}">) outside the conditional and an orphaned
</c:if> remains; fix by turning the empty self-closed <c:if test="${not empty
contributor}"></c:if> into a proper block that wraps the Delete link (i.e.,
place the <a href="<spring:url value='/content/word/delete/${word.id}' />"
...>Delete</a> inside the <c:if test="${not empty contributor}"> ... </c:if>),
ensuring the conditional controls rendering and removing any extra/mismatched
tags.
---
Outside diff comments:
In `@src/main/webapp/WEB-INF/jsp/content/number/edit.jsp`:
- Around line 113-114: The comparison on
numberContributionEvents[0].contributor.id uses contributor.id without guarding
for a null contributor, causing an NPE for unauthenticated users; update the
conditional in edit.jsp to check contributor != null before comparing IDs (i.e.,
ensure contributor is not null && numberContributionEvents[0].contributor.id !=
contributor.id), and make the identical change at the equivalent check in
word/edit.jsp (the conditional around line 173) so both pages safely handle null
contributor sessions.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2361 +/- ##
============================================
+ Coverage 16.80% 16.98% +0.17%
- Complexity 464 468 +4
============================================
Files 264 264
Lines 7836 7836
Branches 899 899
============================================
+ Hits 1317 1331 +14
+ Misses 6445 6432 -13
+ Partials 74 73 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Issue Number
Purpose
Technical Details
Testing Instructions
Screenshots
Format Checks
Note
Files in PRs are automatically checked for format violations with
mvn spotless:check.If this PR contains files with format violations, run
mvn spotless:applyto fix them.