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

Jump to bottom

Preferences: Support setting any dashboard as home, not just the starred ones #54258

Merged
merged 7 commits into from Aug 26, 2022
Merged

Preferences: Support setting any dashboard as home, not just the starred ones #54258

merged 7 commits into from Aug 26, 2022

Conversation

ryantxu
Copy link
Member

@ryantxu ryantxu commented Aug 25, 2022

This PR is really motivated by trying to clean up nested ID use rather than UIDs. The preferences API currently sends both ID and UID.

This PR:

  • replaces the frontend code to work directly with UIDs
  • removes the custom select that limited us to only our starred dashboards
  • updates the backend APIs so that sending an empty string is the same as sending 0

@ryantxu ryantxu added add to changelog no-backport Skip backport of PR labels Aug 25, 2022
@ryantxu ryantxu added this to the 9.1.2 milestone Aug 25, 2022
@ryantxu ryantxu requested a review from a team as a code owner August 25, 2022 20:01
@ryantxu ryantxu requested review from sakjur and yangkb09 and removed request for a team August 25, 2022 20:01
@grafanabot
Copy link
Contributor

Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/31198

@ryantxu ryantxu requested a review from joshhunt as a code owner August 25, 2022 23:43
@ryantxu ryantxu modified the milestones: 9.1.2, 9.2.0 Aug 25, 2022
@grafanabot
Copy link
Contributor

Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/31214

@ryantxu ryantxu mentioned this pull request Aug 26, 2022
Merged
ivanortegaalba
ivanortegaalba reviewed Aug 26, 2022
View reviewed changes
public/app/core/components/SharedPreferences/SharedPreferences.test.tsx
@@ -193,7 +192,7 @@ describe('SharedPreferences', () => {
timezone: 'browser',
weekStart: '',
theme: '',
homeDashboardUID: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

We're removing the Default option from the select, which allows the user to set the default Grafana home page. Is this intentional?

Default value selected Home page when Default is selected After changes, Default doesn't exist
Captura de Pantalla 2022-08-26 a las 10 01 14 Captura de Pantalla 2022-08-26 a las 10 01 05 2022-08-26 10 04 49

Copy link
Member Author

Choose a reason for hiding this comment

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

We now use the standard "x" to clear the custom home dashboard and go back to the default one. I updated the language so it is now:
2022-08-26_08-42-44 (1)

public/app/core/components/SharedPreferences/SharedPreferences.tsx Show resolved Hide resolved
dprokop
dprokop approved these changes Aug 26, 2022
View reviewed changes
Copy link
Member

@dprokop dprokop left a comment

Choose a reason for hiding this comment

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

Looks good and works as expected. Agree with @ivanortegaalba's comments 👍

@grafanabot
Copy link
Contributor

Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/31354

@ryantxu ryantxu enabled auto-merge (squash) August 26, 2022 15:53
@ryantxu ryantxu disabled auto-merge August 26, 2022 16:00
sakjur
sakjur approved these changes Aug 26, 2022
View reviewed changes
Copy link
Contributor

@sakjur sakjur left a comment

Choose a reason for hiding this comment

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

Backend changes LGTM

ivanortegaalba
ivanortegaalba approved these changes Aug 26, 2022
View reviewed changes
Copy link
Contributor

@ivanortegaalba ivanortegaalba left a comment

Choose a reason for hiding this comment

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

All working perfectly well! 💯

@ryantxu ryantxu enabled auto-merge (squash) August 26, 2022 16:10
@ryantxu ryantxu requested a review from a team as a code owner August 26, 2022 16:29
@ryantxu ryantxu merged commit 2db6a19 into main Aug 26, 2022
@ryantxu ryantxu deleted the home-dashboard-setter branch August 26, 2022 16:40
@grafanabot
Copy link
Contributor

Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/31371

ryantxu added a commit that referenced this pull request Aug 29, 2022
@ryantxu
Preferences: Support setting any dashboard as home, not just the star…
1d0b19b 
…red ones (#54258)
@dsotirakis dsotirakis modified the milestones: 9.2.0, 9.2.0-beta1 Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sakjur sakjur sakjur approved these changes

@dprokop dprokop dprokop approved these changes

@ivanortegaalba ivanortegaalba ivanortegaalba approved these changes

@ying-jeanne ying-jeanne Awaiting requested review from ying-jeanne

@yangkb09 yangkb09 Awaiting requested review from yangkb09 yangkb09 was automatically assigned from grafana/backend-platform

@joshhunt joshhunt Awaiting requested review from joshhunt

Assignees
No one assigned
Labels
add to changelog area/backend area/frontend enterprise-failed no-backport Skip backport of PR
Projects
None yet
Milestone
9.2.0-beta1
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@ryantxu @grafanabot @sakjur @dprokop @ivanortegaalba @dsotirakis