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

Navigation: Prevent app crash when importing a dashboard with a uid of home #59874

Merged
merged 3 commits into from Dec 9, 2022

Conversation

ashharrison90
Copy link
Contributor

@ashharrison90 ashharrison90 commented Dec 6, 2022

What is this feature?

  • when a dashboard is starred and added to the navtree, it uses the dashboard uid as the navModel.id: https://github.com/grafana/grafana/blob/main/pkg/services/navtree/navtreeimpl/navtree.go#L345
  • this dashboard uid can then clash with some of our internal navtree ids (e.g. home)
  • add a starred/ prefix for dashboard uids so they can't clash with internal ids
  • also refactor buildInitialState slightly so it can't create a circular reference on the home node
    • before we created this circular reference then removed it after
    • if something clashed with the home node, whatever was processed last is added to the navIndex, so when it comes to remove this circular reference it might find an incorrect node
    • instead, lets remove the home node from the array and handle it separately

Why do we need this feature?

  • prevent crashes in grafana

Who is this feature for?

  • everyone! 🙌

Which issue(s) does this PR fix?:

Fixes #59764

Special notes for your reviewer:

@ashharrison90 ashharrison90 added this to the 9.3.2 milestone Dec 6, 2022
@ashharrison90 ashharrison90 requested a review from a team December 6, 2022 12:01
@ashharrison90 ashharrison90 requested a review from a team as a code owner December 6, 2022 12:01
@ashharrison90 ashharrison90 self-assigned this Dec 6, 2022
@ashharrison90 ashharrison90 requested review from sakjur, zserge and idafurjes and removed request for a team December 6, 2022 12:01
@ashharrison90 ashharrison90 changed the title Navigation: Importing a dashboard with a uid of home no longer crashes the page Navigation: Prevent app crash when importing a dashboard with a uid of home Dec 6, 2022
@ashharrison90 ashharrison90 merged commit a589929 into main Dec 9, 2022
@ashharrison90 ashharrison90 deleted the ash/59764 branch December 9, 2022 12:54
grafanabot pushed a commit that referenced this pull request Dec 9, 2022
…f `home` (#59874)

* change home id to be more unique, refactor so that home circular reference is never created

* prefix starred dashboards

* update reducer

(cherry picked from commit a589929)
ashharrison90 added a commit that referenced this pull request Dec 9, 2022
…h a uid of `home` (#60092)

Navigation: Prevent app crash when importing a dashboard with a uid of `home` (#59874)

* change home id to be more unique, refactor so that home circular reference is never created

* prefix starred dashboards

* update reducer

(cherry picked from commit a589929)

Co-authored-by: Ashley Harrison <ashley.harrison@grafana.com>
GuYounes pushed a commit to paul-wurth/BIXpert that referenced this pull request Feb 8, 2023
…h a uid of `home` (grafana#60092)

Navigation: Prevent app crash when importing a dashboard with a uid of `home` (grafana#59874)

* change home id to be more unique, refactor so that home circular reference is never created

* prefix starred dashboards

* update reducer

(cherry picked from commit a589929)

Co-authored-by: Ashley Harrison <ashley.harrison@grafana.com>
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.

Dashboards: Starring a dashboard where uid = home will throw error for many urls
3 participants