diff --git a/packages/grafana-e2e-selectors/src/selectors/pages.ts b/packages/grafana-e2e-selectors/src/selectors/pages.ts index 93b359e034e..01874d4f9b6 100644 --- a/packages/grafana-e2e-selectors/src/selectors/pages.ts +++ b/packages/grafana-e2e-selectors/src/selectors/pages.ts @@ -638,6 +638,9 @@ export const versionedPages = { saveRefresh: { '11.1.0': 'Dashboard settings Save Dashboard Modal Save refresh checkbox', }, + variablesWarningAlert: { + '12.2.0': 'Dashboard settings Save Dashboard Modal Save variables Variables With Errors Warning Alert', + }, }, SharePanelModal: { linkToRenderedImage: { diff --git a/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.test.tsx b/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.test.tsx index dd4d6734919..94a01df0e17 100644 --- a/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.test.tsx +++ b/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.test.tsx @@ -1,10 +1,11 @@ -import { screen, render } from '@testing-library/react'; +import { screen, render, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { TestProvider } from 'test/helpers/TestProvider'; +import { byTestId, byText } from 'testing-library-selector'; import { selectors } from '@grafana/e2e-selectors'; import { config } from '@grafana/runtime'; -import { sceneGraph, SceneRefreshPicker } from '@grafana/scenes'; +import { ConstantVariable, sceneGraph, SceneRefreshPicker } from '@grafana/scenes'; import { AnnoKeyManagerKind, ManagerKind } from 'app/features/apiserver/types'; import { SaveDashboardResponseDTO } from 'app/types/dashboard'; @@ -27,12 +28,18 @@ jest.mock('app/features/browse-dashboards/api/browseDashboardsAPI', () => ({ useSaveDashboardMutation: () => [saveDashboardMutationMock], })); +const ui = { + saveDashbordText: byText('Save dashboard'), + saveVariablesCheckbox: byTestId(selectors.pages.SaveDashboardModal.saveVariables), + variablesWarningAlert: byTestId(selectors.pages.SaveDashboardModal.variablesWarningAlert), +}; + describe('SaveDashboardDrawer', () => { describe('Given an already saved dashboard', () => { it('should render save drawer with only message textarea', async () => { setup().openAndRender(); - expect(await screen.findByText('Save dashboard')).toBeInTheDocument(); + expect(await ui.saveDashbordText.find()).toBeInTheDocument(); expect(screen.queryByTestId(selectors.pages.SaveDashboardModal.saveTimerange)).not.toBeInTheDocument(); expect(screen.getByText('No changes to save')).toBeInTheDocument(); expect(screen.queryByRole('tab', { name: /Changes/ })).not.toBeInTheDocument(); @@ -50,10 +57,57 @@ describe('SaveDashboardDrawer', () => { openAndRender(); - expect(await screen.findByText('Save dashboard')).toBeInTheDocument(); + expect(await ui.saveDashbordText.find()).toBeInTheDocument(); expect(screen.queryByTestId(selectors.pages.SaveDashboardModal.saveTimerange)).toBeInTheDocument(); }); + it('When variable changed show save variables option', async () => { + const { dashboard, openAndRender } = setup(); + + sceneGraph + .getVariables(dashboard) + .setState({ variables: [new ConstantVariable({ name: 'constant', type: 'constant', value: 'new value' })] }); + + openAndRender(); + + expect(await ui.saveDashbordText.find()).toBeInTheDocument(); + expect(ui.saveVariablesCheckbox.get()).toBeInTheDocument(); + expect(ui.variablesWarningAlert.query()).not.toBeInTheDocument(); // the alert shouldn't show as default + + // checking the checkbox shouldn't show the alert because there are no variables with errors + await userEvent.click(ui.saveVariablesCheckbox.get()); + expect(ui.variablesWarningAlert.query()).not.toBeInTheDocument(); + }); + + it('When variable has error show save variables warning', async () => { + const { dashboard, openAndRender } = setup(); + + sceneGraph.getVariables(dashboard).setState({ + variables: [ + new ConstantVariable({ + name: 'constant', + type: 'constant', + value: 'new value', + error: new Error('Some error'), + }), + ], + }); + + openAndRender(); + + expect(await ui.saveDashbordText.find()).toBeInTheDocument(); + expect(ui.saveVariablesCheckbox.get()).toBeInTheDocument(); + expect(ui.variablesWarningAlert.query()).not.toBeInTheDocument(); // the alert shouldn't show as default + + // checking the save variables checkbox should show the alert + await userEvent.click(ui.saveVariablesCheckbox.get()); + await waitFor(() => expect(ui.variablesWarningAlert.query()).toBeInTheDocument()); + + // unchecking the save variables checkbox should hide the alert + await userEvent.click(ui.saveVariablesCheckbox.get()); + expect(ui.variablesWarningAlert.query()).not.toBeInTheDocument(); + }); + it('Should update diff when including time range is', async () => { const { dashboard, openAndRender } = setup(); @@ -61,7 +115,7 @@ describe('SaveDashboardDrawer', () => { openAndRender(); - expect(await screen.findByText('Save dashboard')).toBeInTheDocument(); + expect(await ui.saveDashbordText.find()).toBeInTheDocument(); expect(screen.queryByTestId(selectors.pages.SaveDashboardModal.saveTimerange)).toBeInTheDocument(); expect(screen.queryByRole('tab', { name: /Changes/ })).not.toBeInTheDocument(); @@ -80,7 +134,7 @@ describe('SaveDashboardDrawer', () => { openAndRender(); - expect(await screen.findByText('Save dashboard')).toBeInTheDocument(); + expect(await ui.saveDashbordText.find()).toBeInTheDocument(); expect(screen.queryByTestId(selectors.pages.SaveDashboardModal.saveRefresh)).toBeInTheDocument(); }); @@ -94,7 +148,7 @@ describe('SaveDashboardDrawer', () => { openAndRender(); - expect(await screen.findByText('Save dashboard')).toBeInTheDocument(); + expect(await ui.saveDashbordText.find()).toBeInTheDocument(); expect(screen.getByTestId(selectors.pages.SaveDashboardModal.saveRefresh)).toBeInTheDocument(); expect(screen.queryByRole('tab', { name: /Changes/ })).not.toBeInTheDocument(); @@ -274,6 +328,15 @@ function setup(overrides?: Partial) { schemaVersion: 30, panels: [], version: 10, + templating: { + list: [ + { + name: 'constant', + query: 'a constant value', + type: 'constant', + }, + ], + }, }, meta: {}, ...overrides, diff --git a/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx b/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx index 10be6f94900..09a45b1bb8c 100644 --- a/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx +++ b/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx @@ -18,6 +18,7 @@ interface SaveDashboardDrawerState extends SceneObjectState { saveVariables?: boolean; saveRefresh?: boolean; saveAsCopy?: boolean; + showVariablesWarning?: boolean; onSaveSuccess?: () => void; } diff --git a/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx b/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx index 076fa78e2e2..f48ec230b0a 100644 --- a/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx +++ b/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx @@ -203,7 +203,12 @@ export interface SaveDashboardFormCommonOptionsProps { } export function SaveDashboardFormCommonOptions({ drawer, changeInfo }: SaveDashboardFormCommonOptionsProps) { - const { saveVariables = false, saveTimeRange = false, saveRefresh = false } = drawer.useState(); + const { + saveVariables = false, + saveTimeRange = false, + saveRefresh = false, + showVariablesWarning = false, + } = drawer.useState(); const { hasTimeChanges, hasVariableValueChanges, hasRefreshChange } = changeInfo; return ( @@ -241,20 +246,38 @@ export function SaveDashboardFormCommonOptions({ drawer, changeInfo }: SaveDashb /> )} {hasVariableValueChanges && ( - + + {saveVariables && showVariablesWarning && ( + + + Some variables failed to load. If you keep “Update default variable values” checked, the current + (failed) values will become the dashboard defaults. You can save anyway or uncheck the option to avoid + storing those (failed) values. + + )} - description={t( - 'dashboard-scene.save-dashboard-form-common-options.save-variables-description-current-values-default', - 'Will make the current values the new default' - )} - checked={saveVariables} - onChange={drawer.onToggleSaveVariables} - data-testid={selectors.pages.SaveDashboardModal.saveVariables} - /> + )} ); diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.tsx index a8a863a8c4c..cf2a6ef2943 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardScene.tsx @@ -414,6 +414,7 @@ export class DashboardScene extends SceneObjectBase impleme dashboardRef: this.getRef(), saveAsCopy, onSaveSuccess, + showVariablesWarning: this.hasVariableErrors(), }), }); } @@ -792,6 +793,10 @@ export class DashboardScene extends SceneObjectBase impleme getPath() { return this.state.meta.k8s?.annotations?.[AnnoKeySourcePath]; } + + private hasVariableErrors(): boolean { + return Boolean(this.state.$variables?.state.variables.find((v) => Boolean(v.state.error))); + } } export class DashboardVariableDependency implements SceneVariableDependencyConfigLike { diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 86c5e18ab7d..7a4beab2952 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -6096,7 +6096,9 @@ "save-timerange-description-current-range-default": "Will make current time range the new default", "save-timerange-label-update-default-time-range": "Update default time range", "save-variables-description-current-values-default": "Will make the current values the new default", - "save-variables-label-update-default-variable-values": "Update default variable values" + "save-variables-label-update-default-variable-values": "Update default variable values", + "show-variables-warning-alert-body": "Some variables failed to load. If you keep “Update default variable values” checked, the current (failed) values will become the dashboard defaults. You can save anyway or uncheck the option to avoid storing those (failed) values.", + "show-variables-warning-alert-title": "Variable queries failed" }, "save-library-viz-panel-modal": { "affected-dashboards_one": "This update will affect <1>{{count}} dashboards. The following dashboards using the panel will be affected:",