Make Audit only show up for Manager role#52
Conversation
…and make Audit only show up for Manager role
📝 WalkthroughWalkthroughUpdated Thymeleaf conditional logic in the header template to change role verification from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.
🧹 Nitpick comments (1)
src/main/resources/templates/fragments/header.html (1)
14-21: Reduce duplicated role checks and prefer enum-stable comparison.Lines 14, 17, and 20 repeat the same condition. Replace
role().toString()withrole().name()for enum comparison—name()returns the stable enum constant name and cannot be overridden, whereastoString()can be customized and may cause issues if changed. Additionally, wrap these three links in a singleth:blockto avoid repeating the entire condition:♻️ Suggested refactor
- <a class="nav-link" - th:if="${currentActor != null && currentActor.role() != null && currentActor.role().toString() == 'MANAGER'}" - href="/ui/audit">Audit</a> - <a class="nav-link" - th:if="${currentActor != null && currentActor.role() != null && currentActor.role().toString() == 'MANAGER'}" - href="/ui/patients">Manage Patients</a> - <a class="nav-link" - th:if="${currentActor != null && currentActor.role() != null && currentActor.role().toString() == 'MANAGER'}" - href="/ui/employees">Manage Users</a></nav> + <th:block th:if="${currentActor != null && currentActor.role() != null && currentActor.role().name() == 'MANAGER'}"> + <a class="nav-link" href="/ui/audit">Audit</a> + <a class="nav-link" href="/ui/patients">Manage Patients</a> + <a class="nav-link" href="/ui/employees">Manage Users</a> + </th:block> + </nav>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/templates/fragments/header.html` around lines 14 - 21, The three nav links repeat the same null-and-role check using role().toString(), so replace the repeated checks by wrapping the three <a> links in a single th:block with the consolidated condition (currentActor != null && currentActor.role() != null && currentActor.role().name() == 'MANAGER'), change each per-link th:if to no condition (remove per-link checks), and use role().name() instead of role().toString() to ensure stable enum comparison for currentActor.role().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/resources/templates/fragments/header.html`:
- Around line 14-21: The three nav links repeat the same null-and-role check
using role().toString(), so replace the repeated checks by wrapping the three
<a> links in a single th:block with the consolidated condition (currentActor !=
null && currentActor.role() != null && currentActor.role().name() == 'MANAGER'),
change each per-link th:if to no condition (remove per-link checks), and use
role().name() instead of role().toString() to ensure stable enum comparison for
currentActor.role().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a4276778-1452-4494-99b8-dd68570ada58
📒 Files selected for processing (1)
src/main/resources/templates/fragments/header.html
And Update role checks in header to use
toString()instead ofname()Summary by CodeRabbit