diff --git a/.betterer.results b/.betterer.results index 948e4dab4bd..bab1eb40406 100644 --- a/.betterer.results +++ b/.betterer.results @@ -2401,6 +2401,10 @@ exports[`better eslint`] = { [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"], [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "1"] ], + "public/app/features/dashboard-scene/saving/getSaveDashboardChange.ts:5381": [ + [0, 0, 0, "Do not use any type assertions.", "0"], + [0, 0, 0, "Do not use any type assertions.", "1"] + ], "public/app/features/dashboard-scene/saving/shared.tsx:5381": [ [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"] ], diff --git a/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx b/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx index 5598d6e8583..8855015a913 100644 --- a/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx +++ b/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx @@ -8,7 +8,7 @@ import { DashboardScene } from '../scene/DashboardScene'; import { SaveDashboardAsForm } from './SaveDashboardAsForm'; import { SaveDashboardForm } from './SaveDashboardForm'; -import { getSaveDashboardChange } from './shared'; +import { getSaveDashboardChange } from './getSaveDashboardChange'; interface SaveDashboardDrawerState extends SceneObjectState { dashboardRef: SceneObjectRef<DashboardScene>; @@ -28,12 +28,12 @@ export class SaveDashboardDrawer extends SceneObjectBase<SaveDashboardDrawerStat }; public onToggleSaveVariables = () => { - this.setState({ saveTimeRange: !this.state.saveTimeRange }); + this.setState({ saveVariables: !this.state.saveVariables }); }; static Component = ({ model }: SceneComponentProps<SaveDashboardDrawer>) => { - const { showDiff, saveAsCopy, saveTimeRange } = model.useState(); - const changeInfo = getSaveDashboardChange(model.state.dashboardRef.resolve(), saveTimeRange); + const { showDiff, saveAsCopy, saveTimeRange, saveVariables } = model.useState(); + const changeInfo = getSaveDashboardChange(model.state.dashboardRef.resolve(), saveTimeRange, saveVariables); const { changedSaveModel, initialSaveModel, diffs, diffCount } = changeInfo; const dashboard = model.state.dashboardRef.resolve(); diff --git a/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx b/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx index 8601cd0afe0..d931944a7c4 100644 --- a/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx +++ b/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx @@ -25,7 +25,7 @@ export interface Props { export function SaveDashboardForm({ dashboard, drawer, changeInfo }: Props) { const { saveVariables = false, saveTimeRange = false } = drawer.useState(); - const { changedSaveModel, hasChanges, hasTimeChanged, hasVariableValuesChanged } = changeInfo; + const { changedSaveModel, hasChanges, hasTimeChanges, hasVariableValueChanges } = changeInfo; const { state, onSaveDashboard } = useDashboardSave(false); const [options, setOptions] = useState<SaveDashboardOptions>({ @@ -102,7 +102,7 @@ export function SaveDashboardForm({ dashboard, drawer, changeInfo }: Props) { return ( <Stack gap={0} direction="column"> - {hasTimeChanged && ( + {hasTimeChanges && ( <Field label="Save current time range" description="Will make current time range the new default"> <Checkbox id="save-timerange" @@ -112,12 +112,12 @@ export function SaveDashboardForm({ dashboard, drawer, changeInfo }: Props) { /> </Field> )} - {hasVariableValuesChanged && ( + {hasVariableValueChanges && ( <Field label="Save current variable values" description="Will make the current values the new default"> <Checkbox + id="save-variables" checked={saveVariables} onChange={drawer.onToggleSaveVariables} - label="Save current variable values as dashboard default" aria-label={selectors.pages.SaveDashboardModal.saveVariables} /> </Field> diff --git a/public/app/features/dashboard-scene/saving/getSaveDashboardChange.test.ts b/public/app/features/dashboard-scene/saving/getSaveDashboardChange.test.ts new file mode 100644 index 00000000000..d443d83ba68 --- /dev/null +++ b/public/app/features/dashboard-scene/saving/getSaveDashboardChange.test.ts @@ -0,0 +1,92 @@ +import { MultiValueVariable, sceneGraph } from '@grafana/scenes'; + +import { transformSaveModelToScene } from '../serialization/transformSaveModelToScene'; +import { transformSceneToSaveModel } from '../serialization/transformSceneToSaveModel'; + +import { getSaveDashboardChange } from './getSaveDashboardChange'; + +describe('getSaveDashboardChange', () => { + it('Can detect no changes', () => { + const dashboard = setup(); + const result = getSaveDashboardChange(dashboard, false); + expect(result.hasChanges).toBe(false); + expect(result.diffCount).toBe(0); + }); + + it('Can detect time changed', () => { + const dashboard = setup(); + + sceneGraph.getTimeRange(dashboard).setState({ from: 'now-1h', to: 'now' }); + + const result = getSaveDashboardChange(dashboard, false); + expect(result.hasChanges).toBe(false); + expect(result.diffCount).toBe(0); + expect(result.hasTimeChanges).toBe(true); + }); + + it('Can save time change', () => { + const dashboard = setup(); + + sceneGraph.getTimeRange(dashboard).setState({ from: 'now-1h', to: 'now' }); + + const result = getSaveDashboardChange(dashboard, true); + expect(result.hasChanges).toBe(true); + expect(result.diffCount).toBe(1); + }); + + it('Can detect variable change', () => { + const dashboard = setup(); + + const appVar = sceneGraph.lookupVariable('app', dashboard) as MultiValueVariable; + appVar.changeValueTo('app2'); + + const result = getSaveDashboardChange(dashboard, false, false); + + expect(result.hasVariableValueChanges).toBe(true); + expect(result.hasChanges).toBe(false); + expect(result.diffCount).toBe(0); + }); + + it('Can save variable value change', () => { + const dashboard = setup(); + + const appVar = sceneGraph.lookupVariable('app', dashboard) as MultiValueVariable; + appVar.changeValueTo('app2'); + + const result = getSaveDashboardChange(dashboard, false, true); + + expect(result.hasVariableValueChanges).toBe(true); + expect(result.hasChanges).toBe(true); + expect(result.diffCount).toBe(2); + }); +}); + +function setup() { + const dashboard = transformSaveModelToScene({ + dashboard: { + title: 'hello', + uid: 'my-uid', + schemaVersion: 30, + panels: [], + version: 10, + templating: { + list: [ + { + name: 'app', + type: 'custom', + current: { + text: 'app1', + value: 'app1', + }, + }, + ], + }, + }, + meta: {}, + }); + + const initialSaveModel = transformSceneToSaveModel(dashboard); + dashboard.setInitialSaveModel(initialSaveModel); + + return dashboard; +} diff --git a/public/app/features/dashboard-scene/saving/getSaveDashboardChange.ts b/public/app/features/dashboard-scene/saving/getSaveDashboardChange.ts new file mode 100644 index 00000000000..86f771a1f50 --- /dev/null +++ b/public/app/features/dashboard-scene/saving/getSaveDashboardChange.ts @@ -0,0 +1,83 @@ +import { isEqual } from 'lodash'; + +import { AdHocVariableModel, TypedVariableModel } from '@grafana/data'; +import { Dashboard } from '@grafana/schema'; + +import { DashboardScene } from '../scene/DashboardScene'; +import { transformSceneToSaveModel } from '../serialization/transformSceneToSaveModel'; +import { jsonDiff } from '../settings/version-history/utils'; + +import { DashboardChangeInfo } from './shared'; + +export function getSaveDashboardChange( + dashboard: DashboardScene, + saveTimeRange?: boolean, + saveVariables?: boolean +): DashboardChangeInfo { + const initialSaveModel = dashboard.getInitialSaveModel()!; + const changedSaveModel = transformSceneToSaveModel(dashboard); + const hasTimeChanged = getHasTimeChanged(changedSaveModel, initialSaveModel); + + const hasVariableValueChanges = applyVariableChanges(changedSaveModel, initialSaveModel, saveVariables); + + if (!saveTimeRange) { + changedSaveModel.time = initialSaveModel.time; + } + + const diff = jsonDiff(initialSaveModel, changedSaveModel); + + let diffCount = 0; + for (const d of Object.values(diff)) { + diffCount += d.length; + } + + return { + changedSaveModel, + initialSaveModel, + diffs: diff, + diffCount, + hasChanges: diffCount > 0, + hasTimeChanges: hasTimeChanged, + isNew: changedSaveModel.version === 0, + hasVariableValueChanges, + }; +} + +export function getHasTimeChanged(saveModel: Dashboard, originalSaveModel: Dashboard) { + return saveModel.time?.from !== originalSaveModel.time?.from || saveModel.time?.to !== originalSaveModel.time?.to; +} + +export function applyVariableChanges(saveModel: Dashboard, originalSaveModel: Dashboard, saveVariables?: boolean) { + const originalVariables = originalSaveModel.templating?.list ?? []; + const variablesToSave = saveModel.templating?.list ?? []; + let hasVariableValueChanges = false; + + for (const variable of variablesToSave) { + const original = originalVariables.find(({ name, type }) => name === variable.name && type === variable.type); + + if (!original) { + continue; + } + + // Old schema property that never should be in persisted model + if (original.current && Object.hasOwn(original.current, 'selected')) { + delete original.current.selected; + } + + if (!isEqual(variable.current, original.current)) { + hasVariableValueChanges = true; + } + + if (!saveVariables) { + const typed = variable as TypedVariableModel; + if (typed.type === 'adhoc') { + typed.filters = (original as AdHocVariableModel).filters; + } else { + variable.current = original.current; + variable.options = original.options; + } + } + } + + return hasVariableValueChanges; +} diff --git a/public/app/features/dashboard-scene/saving/shared.tsx b/public/app/features/dashboard-scene/saving/shared.tsx index 3a836ede0a8..ad52947cb44 100644 --- a/public/app/features/dashboard-scene/saving/shared.tsx +++ b/public/app/features/dashboard-scene/saving/shared.tsx @@ -5,9 +5,7 @@ import { isFetchError } from '@grafana/runtime'; import { Dashboard } from '@grafana/schema'; import { Alert, Box, Button, Stack } from '@grafana/ui'; -import { DashboardScene } from '../scene/DashboardScene'; -import { transformSceneToSaveModel } from '../serialization/transformSceneToSaveModel'; -import { Diffs, jsonDiff } from '../settings/version-history/utils'; +import { Diffs } from '../settings/version-history/utils'; export interface DashboardChangeInfo { changedSaveModel: Dashboard; @@ -15,48 +13,11 @@ export interface DashboardChangeInfo { diffs: Diffs; diffCount: number; hasChanges: boolean; - hasTimeChanged: boolean; - hasVariableValuesChanged: boolean; + hasTimeChanges: boolean; + hasVariableValueChanges: boolean; isNew?: boolean; } -export function getSaveDashboardChange(dashboard: DashboardScene, saveTimeRange?: boolean): DashboardChangeInfo { - const initialSaveModel = dashboard.getInitialSaveModel()!; - const changedSaveModel = transformSceneToSaveModel(dashboard); - const hasTimeChanged = getHasTimeChanged(changedSaveModel, initialSaveModel); - const hasVariableValuesChanged = getVariableValueChanges(changedSaveModel, initialSaveModel); - - if (!saveTimeRange) { - changedSaveModel.time = initialSaveModel.time; - } - - const diff = jsonDiff(initialSaveModel, changedSaveModel); - - let diffCount = 0; - for (const d of Object.values(diff)) { - diffCount += d.length; - } - - return { - changedSaveModel, - initialSaveModel, - diffs: diff, - diffCount, - hasChanges: diffCount > 0, - hasTimeChanged, - isNew: changedSaveModel.version === 0, - hasVariableValuesChanged, - }; -} - -function getHasTimeChanged(saveModel: Dashboard, originalSaveModel: Dashboard) { - return saveModel.time?.from !== originalSaveModel.time?.from || saveModel.time?.to !== originalSaveModel.time?.to; -} - -function getVariableValueChanges(saveModel: Dashboard, originalSaveModel: Dashboard) { - return false; -} - export function isVersionMismatchError(error?: Error) { return isFetchError(error) && error.data && error.data.status === 'version-mismatch'; }