-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix(sidebar): sidebar should only show links user has access to see #2112
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/hospitalrun/hospitalrun-frontend/bpgd4g7zt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks fabulous! Thank you 🙏
Let's have @fox1t check it too, since it's something related to ACL server side feature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should have tests to prove when certain permissions are not on the user, then the links do not render.
Thanks @jackcmeyer, I will attempt to add those in. |
I have pushed some unit tests code. Note though, I thought I only needed to test those scenarios where we do not want to show links since the existing tests, by virtue of them having all privileges setup, are also therefore testing cases where we do want the links to show, i.e. when the user has the correct privileges to access the link(s) in question; I hope that was an okay assumption to make. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👊 🙂
Fixes #2110.
Changes proposed in this pull request: