From 9eb311c037f67aff6073a5c60c9ae295c8d649ac Mon Sep 17 00:00:00 2001 From: Andrew Hackmann <5140848+bossinc@users.noreply.github.com> Date: Wed, 16 Apr 2025 09:07:17 -0500 Subject: [PATCH] Graphite: Fix queries with series refs not updating in alerts (#104019) * use targetFull to see if query has changes for nested series * prettier --- .../datasource/graphite/state/helpers.test.ts | 54 +++++++++++++++++++ .../datasource/graphite/state/helpers.ts | 10 ++-- 2 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 public/app/plugins/datasource/graphite/state/helpers.test.ts diff --git a/public/app/plugins/datasource/graphite/state/helpers.test.ts b/public/app/plugins/datasource/graphite/state/helpers.test.ts new file mode 100644 index 00000000000..9257c83938f --- /dev/null +++ b/public/app/plugins/datasource/graphite/state/helpers.test.ts @@ -0,0 +1,54 @@ +import { handleTargetChanged } from './helpers'; +import { GraphiteQueryEditorState } from './store'; + +describe('handleTargetChanged', () => { + let state: GraphiteQueryEditorState; + + beforeEach(() => { + state = { + queryModel: { + error: null, + target: { target: 'oldTarget' }, + updateModelTarget: jest.fn(), + }, + queries: [], + paused: false, + refresh: jest.fn(), + } as unknown as GraphiteQueryEditorState; + }); + + it('should return early if queryModel.error is set', () => { + state.queryModel.error = new Error('Some error'); + handleTargetChanged(state); + expect(state.queryModel.updateModelTarget).not.toHaveBeenCalled(); + }); + + it('should refresh if target changes and state is not paused', () => { + state.queryModel.target.target = 'oldTarget'; + (state.queryModel.updateModelTarget as jest.Mock).mockImplementation(() => { + state.queryModel.target.target = 'newTarget'; + }); + handleTargetChanged(state); + expect(state.refresh).toHaveBeenCalled(); + }); + + it('should refresh if fullTarget changes and state is not paused', () => { + state.queryModel.target.targetFull = 'oldTargetFull'; + (state.queryModel.updateModelTarget as jest.Mock).mockImplementation(() => { + state.queryModel.target.targetFull = 'newTargetFull'; + }); + handleTargetChanged(state); + expect(state.refresh).toHaveBeenCalled(); + }); + + it('should not refresh if target does not change', () => { + handleTargetChanged(state); + expect(state.refresh).not.toHaveBeenCalled(); + }); + + it('should not refresh if state is paused', () => { + state.paused = true; + handleTargetChanged(state); + expect(state.refresh).not.toHaveBeenCalled(); + }); +}); diff --git a/public/app/plugins/datasource/graphite/state/helpers.ts b/public/app/plugins/datasource/graphite/state/helpers.ts index 4277fc392b2..fd1cb0c91f4 100644 --- a/public/app/plugins/datasource/graphite/state/helpers.ts +++ b/public/app/plugins/datasource/graphite/state/helpers.ts @@ -158,7 +158,8 @@ export function handleTargetChanged(state: GraphiteQueryEditorState): void { return; } - let oldTarget = state.queryModel.target.target; + const oldResolvedTarget = state.queryModel.target.targetFull ?? state.queryModel.target.target; + const oldTargetRemovedSpaces = oldResolvedTarget.replace(/\s+/g, ''); // Interpolate from other queries: // Because of mixed data sources the list may contain queries for non-Graphite data sources. To ensure a valid query // is used for interpolation we should check required properties are passed though in theory it allows to interpolate @@ -167,11 +168,10 @@ export function handleTargetChanged(state: GraphiteQueryEditorState): void { (state.queries || []).filter((query) => 'target' in query && typeof query.target === 'string') ); - // remove spaces from old and new targets - const newTarget = state.queryModel.target.target.replace(/\s+/g, ''); - oldTarget = oldTarget.replace(/\s+/g, ''); + const newResolvedTarget = state.queryModel.target.targetFull ?? state.queryModel.target.target; + const newTargetRemovedSpaces = newResolvedTarget.replace(/\s+/g, ''); - if (newTarget !== oldTarget && !state.paused) { + if (newTargetRemovedSpaces !== oldTargetRemovedSpaces && !state.paused) { state.refresh(); } }