Skip to content
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

Snapshot: Show proper breadcrumb path #98806

Merged
merged 2 commits into from
Jan 10, 2025
Merged

Snapshot: Show proper breadcrumb path #98806

merged 2 commits into from
Jan 10, 2025

Conversation

ashharrison90
Copy link
Contributor

@ashharrison90 ashharrison90 commented Jan 10, 2025

What is this feature?

  • moving to scenes regressed the breadcrumbs in dashboards
  • updates the getDashboardUrl function to account for whether the dashboard is a snapshot or not
  • adjusts the navModel in DashboardSceneRenderer to also check if the dashboard is a snapshot and set the section to Snapshots correctly

Why do we need this feature?

  • show proper breadcrumb path when viewing a snapshot

Who is this feature for?

  • anyone using snapshots

Which issue(s) does this PR fix?:

Fixes #96830

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@ashharrison90 ashharrison90 added this to the 11.5.x milestone Jan 10, 2025
@ashharrison90 ashharrison90 requested review from a team January 10, 2025 12:50
@ashharrison90 ashharrison90 self-assigned this Jan 10, 2025
@ashharrison90 ashharrison90 requested a review from a team as a code owner January 10, 2025 12:50
@ashharrison90 ashharrison90 requested review from joshhunt, eledobleefe, Sergej-Vlasov, kaydelaney, evictorero and AgnesToulet and removed request for a team January 10, 2025 12:50
Copy link
Contributor

@evictorero evictorero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thanks for taking care of this, tested and looks good.
Probably not part of this PR but one thing to notice is this won't fix the unauthenticated view of a snapshot. Opening the same Snapshot in an incognito window will still display the Page not found in the breadcrumb.

@ashharrison90
Copy link
Contributor Author

Great! Thanks for taking care of this, tested and looks good. Probably not part of this PR but one thing to notice is this won't fix the unauthenticated view of a snapshot. Opening the same Snapshot in an incognito window will still display the Page not found in the breadcrumb.

yep for sure, that's captured in #96988

it's more general than just snapshots anyway. it's a generic access problem of "what should we show if a user doesn't have permission to a certain resource"? right now we just show "Not found" which isn't great, but at least it isn't leaking any details that should be restricted 😅

@ashharrison90 ashharrison90 merged commit ee362ed into main Jan 10, 2025
22 checks passed
@ashharrison90 ashharrison90 deleted the ash/96830 branch January 10, 2025 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Breadcrumbs: snapshot just shows Home breadcrumb after user logged in
3 participants