-
Notifications
You must be signed in to change notification settings - Fork 263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hide version in footer for unauthenticated users #2683
Conversation
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.
This looks reasonable enough, although as @Kircheneer pointed out, there's still the unauthenticated /api/status/
endpoint to consider from a security standpoint (#1610).
Would you be willing to add a unit test for this rendering logic, probably in nautobot.core.tests.test_views.HomeViewTestCase
?
That was not my experience, see my comment in the issue. Sure, the devcontainer seems broken for VSCode (requires "db" but db is not defined, amongst other things). I've pushed a commit with a test, let's hope I nailed it, but noone usually does on the first try 😅 |
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.
Looks good to me. We might want to revisit the test case at some point to make it more robust but it's definitely good enough for now. Thank you!
Sorry for the re-reviews. The CI only runs partial jobs since I'm a new contributor. |
Closes: #2682
What's Changed
TODO
Before
After