Dashboard: Migration - Dashboard Settings Variables (List, Duplicate, Delete) #78917

axelavargas · 2023-11-30T16:22:09Z

Basic infrastructure

Give feedback

One view dashboard variables list

One view dashboard variables list
Options
Successfully updated the issue's project

There was an error updating the issue's project
One can Delete, Duplicate a variable

One can Delete, Duplicate a variable
Options
Successfully updated the issue's project

There was an error updating the issue's project
Deleting a variable shows a confirmation modal

Deleting a variable shows a confirmation modal
Options
Successfully updated the issue's project

There was an error updating the issue's project
Variable(s) changes are reflected in a Dashboard Scene

Variable(s) changes are reflected in a Dashboard Scene
Options
Successfully updated the issue's project

There was an error updating the issue's project
Add Unit Test

Add Unit Test
Options
Successfully updated the issue's project

There was an error updating the issue's project
Options

NOTE:

Not implemented anything regarding usage, renaming variables or creating variables

Part of: #78690

…les with the old format

…lete variables

dprokop · 2023-12-11T10:58:48Z

Did a quick testing @axelavargas, and found some issues:

In a dashboard without variables, creating a variable ain't possible
In a dashboard without variables, refreshing variables page results in the following error:

If there are no variables in a dashboard, the variables table is still shown i think:

Deleting a variable seems not to work.

public/app/features/dashboard-scene/settings/VariablesEditView.tsx

axelavargas · 2023-12-11T12:40:22Z

@dprokop Thanks for the review 🙌🏾 , regarding not being able to create variables, indeed that is not being implemented in this PR. I am planning to tackle it in a different PR, my reasoning was to keep this PR small and also learn that we will need to create new forms per each variable type.

public/app/features/dashboard-scene/settings/VariablesEditView.tsx

ivanortegaalba · 2023-12-12T13:59:40Z

public/app/features/dashboard-scene/settings/variables/VariableEditorList.tsx

+        infoBox={{
+          __html: ` <p>
+                    Variables enable more interactive and dynamic dashboards. Instead of hard-coding things like server
+                    or sensor names in your metric queries you can use variables in their place. Variables are shown as
+                    list boxes at the top of the dashboard. These drop-down lists make it easy to change the data
+                    being displayed in your dashboard. Check out the
+                    <a class="external-link" href="https://grafana.com/docs/grafana/latest/variables/" target="_blank">
+                      Templates and variables documentation
+                    </a>
+                    for more information.
+                  </p>`,
+        }}


Probably, we should add internationalization here. WDYT?

I think is a good idea, but I would refrain from doing it in the scope of the migration 😄.

ivanortegaalba · 2023-12-13T16:27:46Z

public/app/features/dashboard-scene/serialization/transformSaveModelToScene.ts

@@ -195,6 +195,11 @@ export function createDashboardSceneFromDashboardModel(oldModel: DashboardModel)
    variables = new SceneVariableSet({
      variables: variableObjects,
    });
+  } else {


If it is mandatory to get always a valid value in variables, probably it is a good idea to add a test for it. WDYT?

Good idea, I have added the test :)

yeah, +1 to ivan's. Overall what we see is it's better to attach even empty objects rather than conditonally adding theme. It simplifies downstream code a lot.

Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

public/app/features/dashboard-scene/settings/VariablesEditView.tsx

dprokop

Looks good now @axelavargas. I suggest merging this PR and moving forward with the following tasks :) They may reveal some gaps that we may not see here, but not necessarily.

ivanortegaalba

Thanks for considering the feedback ✅

First attempt: List variables using SceneVariablesSet and also variab…

bdc6235

…les with the old format

axelavargas added area/dashboard no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels Nov 30, 2023

axelavargas self-assigned this Nov 30, 2023

grafana-delivery-bot bot added this to the 10.3.x milestone Nov 30, 2023

grafana-pr-automation bot added the area/frontend label Nov 30, 2023

axelavargas added 9 commits December 6, 2023 14:29

Merge remote-tracking branch 'origin' into axelav/settings-variables

bbf200b

Merge remote-tracking branch 'origin' into axelav/settings-variables

16016d6

List variables and create Scaffold for Change order, duplicate and de…

129527d

…lete variables

Refactor: use right methods to get variables and implement onDelete

3fe0edf

Implement duplicate variables and clean up code

cb1ce5a

Modify duplicated function to keep the same format as core grafana

8d92f02

Fix betterer and linter issues

474e46b

Add confirmation delete modal

640e818

Fix linter

87c2297

axelavargas marked this pull request as ready for review December 8, 2023 15:03

axelavargas requested a review from a team as a code owner December 8, 2023 15:03

axelavargas requested review from dprokop, kaydelaney and ivanortegaalba and removed request for a team December 8, 2023 15:03

axelavargas added 5 commits December 8, 2023 16:05

More linter...

9e19e38

Fix React removal

33d3040

Merge remote-tracking branch 'origin' into axelav/settings-variables

55d1822

Attempt to fix scenes types

8b42494

Fix typescript error, pass variableScene to the component

0be281c

dprokop reviewed Dec 11, 2023

View reviewed changes

public/app/features/dashboard-scene/settings/VariablesEditView.tsx Outdated Show resolved Hide resolved

axelavargas changed the title ~~Dashboard: Migration - Dashboard Settings Variables Basic Infra~~ Dashboard: Migration - Dashboard Settings Variables (List, Duplicate, Delete) Dec 11, 2023

axelavargas added 3 commits December 12, 2023 10:24

Disable create button and empty variable set

5db6c48

Move methods to VariableEditView model

aca9c65

Add unit test

f99ea12

axelavargas requested a review from dprokop December 13, 2023 10:09

axelavargas added 2 commits December 13, 2023 11:26

Merge remote-tracking branch 'origin' into axelav/settings-variables

3052d19

Fix linter

db3e7a9

dprokop requested a review from torkelo December 13, 2023 14:18

torkelo reviewed Dec 13, 2023

View reviewed changes

axelavargas added 2 commits December 13, 2023 17:48

Use getDashboardSceneFor method in all DashboardSettings

db8ed9d

Remove dashboardRef param from Dashboard Scene Url Sync

dfea723

ivanortegaalba reviewed Dec 13, 2023

View reviewed changes

axelavargas and others added 3 commits December 13, 2023 17:59

Add missing changes

bcd2a52

Apply suggestions from code review

dccd570

Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

Fix references of PR suggestion

3d835c2

axelavargas requested review from ivanortegaalba and torkelo December 13, 2023 17:32

Add unit test for default variable set

811347a

torkelo reviewed Dec 13, 2023

View reviewed changes

public/app/features/dashboard-scene/settings/VariablesEditView.tsx Outdated Show resolved Hide resolved

Remove unnecesary function

0c4f50d

dprokop reviewed Dec 14, 2023

View reviewed changes

public/app/features/dashboard-scene/settings/VariablesEditView.tsx Outdated Show resolved Hide resolved

dprokop approved these changes Dec 14, 2023

View reviewed changes

Remove unnecesary get _dashboard function

7edc34a

axelavargas requested a review from torkelo December 14, 2023 11:14

torkelo approved these changes Dec 14, 2023

View reviewed changes

ivanortegaalba approved these changes Dec 14, 2023

View reviewed changes

axelavargas merged commit 4c6bbab into main Dec 14, 2023
15 checks passed

axelavargas deleted the axelav/settings-variables branch December 14, 2023 11:54

summerwollin modified the milestones: 10.3.x, 10.3.0 Jan 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dashboard: Migration - Dashboard Settings Variables (List, Duplicate, Delete) #78917

Dashboard: Migration - Dashboard Settings Variables (List, Duplicate, Delete) #78917

axelavargas commented Nov 30, 2023 •

edited

Basic infrastructure

dprokop commented Dec 11, 2023

axelavargas commented Dec 11, 2023 •

edited

ivanortegaalba Dec 12, 2023

axelavargas Dec 13, 2023

ivanortegaalba Dec 13, 2023

axelavargas Dec 13, 2023

dprokop Dec 14, 2023

dprokop left a comment

ivanortegaalba left a comment

Dashboard: Migration - Dashboard Settings Variables (List, Duplicate, Delete) #78917

Dashboard: Migration - Dashboard Settings Variables (List, Duplicate, Delete) #78917

Conversation

axelavargas commented Nov 30, 2023 • edited

Basic infrastructure

NOTE:

dprokop commented Dec 11, 2023

axelavargas commented Dec 11, 2023 • edited

ivanortegaalba Dec 12, 2023

Choose a reason for hiding this comment

axelavargas Dec 13, 2023

Choose a reason for hiding this comment

ivanortegaalba Dec 13, 2023

Choose a reason for hiding this comment

axelavargas Dec 13, 2023

Choose a reason for hiding this comment

dprokop Dec 14, 2023

Choose a reason for hiding this comment

dprokop left a comment

Choose a reason for hiding this comment

ivanortegaalba left a comment

Choose a reason for hiding this comment

axelavargas commented Nov 30, 2023 •

edited

axelavargas commented Dec 11, 2023 •

edited