fix(dashboard): hide sidebar on cluster-level pages when no tenant selected#2106
fix(dashboard): hide sidebar on cluster-level pages when no tenant selected#2106
Conversation
…lected Remove stock-instance-* sidebars that were populated with namespace-scoped menu items, causing the sidebar to incorrectly appear on cluster-level pages. Clear the sidebar fallback ID so the frontend gracefully renders no sidebar when no matching sidebar resource exists. Co-Authored-By: Claude <[email protected]> Signed-off-by: Kirill Ilin <[email protected]>
📝 WalkthroughWalkthroughThe PR removes deprecated stock-instance sidebar identifiers from the dashboard management system, consolidating sidebar resource configuration to use only stock-project sidebars and removing the sidebar customization fallback ID reference. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
Summary of ChangesHello @sircthulhu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the dashboard sidebar displayed incorrect namespace-scoped menu items on cluster-level pages before a tenant was selected, leading to broken URLs. The changes ensure that the sidebar remains empty in this state, providing a cleaner and more functional user experience by preventing the creation of inappropriate sidebar resources and configuring the frontend to hide the sidebar when no tenant is selected. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses the issue of the sidebar being incorrectly displayed on cluster-level pages by removing the stock-instance-* sidebar resources and clearing the fallback sidebar ID. The changes are logical and align with the description. I've also identified a related issue in manager.go where the list of expected sidebar resources for cleanup is incomplete, which could lead to the incorrect deletion of other static sidebars. I've added a suggestion to fix this.
sircthulhu
requested review from
IvanHunters,
androndo,
kvaps,
lexfrei and
lllamnyp
as code owners
dosubot
bot
added
size:S
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/controller/dashboard/sidebar.go (1)
283-284: Stale comment references removed sidebar type.The comment mentions
stock-instance-*sidebars, but these are no longer created after this PR. Consider updating the comment to reflect the current behavior.📝 Suggested comment update
- // Only set owner reference for dynamic sidebars (stock-project-factory-{kind}-details) - // Static sidebars (stock-instance-*, stock-project-*) should not have owner references + // Only set owner reference for dynamic sidebars (stock-project-factory-{kind}-details) + // Static sidebars (stock-project-*) should not have owner references🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/dashboard/sidebar.go` around lines 283 - 284, Update the stale comment in internal/controller/dashboard/sidebar.go that currently reads "Only set owner reference for dynamic sidebars (stock-project-factory-{kind}-details) // Static sidebars (stock-instance-*, stock-project-*) should not have owner references" to remove the obsolete "stock-instance-*" reference and reflect current behavior; keep the mention of dynamic sidebar pattern "stock-project-factory-{kind}-details" and the rule about static sidebars not receiving owner references so the comment accurately documents the code's intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/controller/dashboard/sidebar.go`:
- Around line 283-284: Update the stale comment in
internal/controller/dashboard/sidebar.go that currently reads "Only set owner
reference for dynamic sidebars (stock-project-factory-{kind}-details) // Static
sidebars (stock-instance-*, stock-project-*) should not have owner references"
to remove the obsolete "stock-instance-*" reference and reflect current
behavior; keep the mention of dynamic sidebar pattern
"stock-project-factory-{kind}-details" and the rule about static sidebars not
receiving owner references so the comment accurately documents the code's
intent.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/controller/dashboard/manager.gointernal/controller/dashboard/sidebar.gopackages/system/dashboard/templates/web.yaml
💤 Files with no reviewable changes (1)
- internal/controller/dashboard/manager.go
…ages PR #2106 removed stock-instance-* sidebar resources to fix broken URLs on the main page before namespace selection. However, these sidebars are required for rendering namespace-level pages (api-table, api-form, etc.) such as the Backup Plans page. Without stock-instance-api-table, the frontend cannot find the sidebar for namespace-scoped api-table pages and renders an empty page instead. The original bug (broken URLs with empty namespace placeholder) is already fixed by CUSTOMIZATION_SIDEBAR_FALLBACK_ID="" in web.yaml, so re-adding stock-instance-* sidebars does not reintroduce it. Co-Authored-By: Claude <[email protected]> Signed-off-by: Kirill Ilin <[email protected]>
…pages (#2136) ## What this PR does Restores `stock-instance-api-form`, `stock-instance-api-table`, `stock-instance-builtin-form`, and `stock-instance-builtin-table` sidebar resources that were removed in #2106, and adds them back to the orphan cleanup allowlist. PR #2106 removed these sidebars to fix broken URLs on the main page before namespace selection (`default//api-table/...`). However, `stock-instance-*` sidebars are required by the frontend for namespace-level api-table/api-form pages. Without them and with `CUSTOMIZATION_SIDEBAR_FALLBACK_ID=""`, the frontend cannot find a sidebar for pages like Backup Plans and renders an empty page where no interaction is possible. The broken-URL bug is already fully fixed by `CUSTOMIZATION_SIDEBAR_FALLBACK_ID=""` in `web.yaml`. Re-adding `stock-instance-*` does not reintroduce it, since these sidebars are only shown when the user is on a namespace-level page where the `{namespace}` placeholder is filled. ### Release note ```release-note [dashboard] fix empty page on Backup Plans and other namespace-level api-table pages ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added four new dashboard sidebar resources for stock instances: API form, API table, built-in form, and built-in table views. These enable expanded dashboard customization options for managing stock instance configurations and data. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…ages PR #2106 removed stock-instance-* sidebar resources to fix broken URLs on the main page before namespace selection. However, these sidebars are required for rendering namespace-level pages (api-table, api-form, etc.) such as the Backup Plans page. Without stock-instance-api-table, the frontend cannot find the sidebar for namespace-scoped api-table pages and renders an empty page instead. The original bug (broken URLs with empty namespace placeholder) is already fixed by CUSTOMIZATION_SIDEBAR_FALLBACK_ID="" in web.yaml, so re-adding stock-instance-* sidebars does not reintroduce it. Co-Authored-By: Claude <[email protected]> Signed-off-by: Kirill Ilin <[email protected]> (cherry picked from commit 45b61f8)
2 participants
What this PR does
In the current version, the sidebar incorrectly shows namespace-scoped menu items on cluster-level pages (before a tenant is selected). Clicking these items produces broken URLs with double
//(e.g.default//api-table/backups.cozystack.io/...) because the{namespace}placeholder resolves to an empty string.This PR fixes the issue by:
CUSTOMIZATION_SIDEBAR_FALLBACK_IDenv var so the frontend renders no sidebar when no matching sidebar resource existsScreenshot after changes
Test plan
//in sidebar URLsgo test ./internal/controller/dashboard/...Release note
Summary by CodeRabbit