From ed3fb71f7b49c079dd05c626cf323316b7b3f0e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 14 Sep 2023 16:12:20 +0200 Subject: [PATCH] DashboardScene: Panel menu tracking, adding explore menu action and unit tests (#74867) * DashboardScene: Panel menu updates, adding explore action * DashboardScene: Panel menu updates, adding explore action * Fix test * Update test --- packages/grafana-data/src/types/datasource.ts | 2 +- public/app/core/services/keybindingSrv.ts | 6 +- public/app/core/utils/explore.test.ts | 29 ++--- public/app/core/utils/explore.ts | 29 ++--- .../scene/PanelMenuBehavior.test.tsx | 98 ++++++++++++++++ .../scene/PanelMenuBehavior.tsx | 107 +++++++++++------- .../features/dashboard/utils/getPanelMenu.ts | 9 +- .../app/features/explore/state/main.test.ts | 30 ++--- public/app/features/explore/state/main.ts | 12 +- .../plugins/datasource/testdata/datasource.ts | 5 + 10 files changed, 206 insertions(+), 121 deletions(-) create mode 100644 public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx diff --git a/packages/grafana-data/src/types/datasource.ts b/packages/grafana-data/src/types/datasource.ts index 6149b39f337..bbca8f57bb8 100644 --- a/packages/grafana-data/src/types/datasource.ts +++ b/packages/grafana-data/src/types/datasource.ts @@ -339,7 +339,7 @@ abstract class DataSourceApi< getVersion?(optionalOptions?: any): Promise; - interpolateVariablesInQueries?(queries: TQuery[], scopedVars: ScopedVars | {}): TQuery[]; + interpolateVariablesInQueries?(queries: TQuery[], scopedVars: ScopedVars): TQuery[]; /** * An annotation processor allows explicit control for how annotations are managed. diff --git a/public/app/core/services/keybindingSrv.ts b/public/app/core/services/keybindingSrv.ts index f2121150f11..2f126c22145 100644 --- a/public/app/core/services/keybindingSrv.ts +++ b/public/app/core/services/keybindingSrv.ts @@ -11,7 +11,6 @@ import { ShareModal } from 'app/features/dashboard/components/ShareModal'; import { DashboardModel } from 'app/features/dashboard/state'; import { getTimeSrv } from '../../features/dashboard/services/TimeSrv'; -import { getDatasourceSrv } from '../../features/plugins/datasource_srv'; import { RemovePanelEvent, ShiftTimeEvent, @@ -261,8 +260,9 @@ export class KeybindingSrv { this.bindWithPanelId('p x', async (panelId) => { const panel = dashboard.getPanelById(panelId)!; const url = await getExploreUrl({ - panel, - datasourceSrv: getDatasourceSrv(), + queries: panel.targets, + dsRef: panel.datasource, + scopedVars: panel.scopedVars, timeRange: getTimeSrv().timeRange(), }); diff --git a/public/app/core/utils/explore.test.ts b/public/app/core/utils/explore.test.ts index e6e23006f85..38eaac8eb5b 100644 --- a/public/app/core/utils/explore.test.ts +++ b/public/app/core/utils/explore.test.ts @@ -71,31 +71,20 @@ describe('state functions', () => { describe('getExploreUrl', () => { const args = { - panel: { - getSavedId: () => 1, - targets: [ - { refId: 'A', expr: 'query1', legendFormat: 'legendFormat1' }, - { refId: 'B', expr: 'query2', datasource: { type: '__expr__', uid: '__expr__' } }, - ], - }, - datasourceSrv: { - get() { - return { - getRef: jest.fn(), - }; - }, - getDataSourceById: jest.fn(), + queries: [ + { refId: 'A', expr: 'query1', legendFormat: 'legendFormat1' }, + { refId: 'B', expr: 'query2', datasource: { type: '__expr__', uid: '__expr__' } }, + ], + dsRef: { + uid: 'ds1', }, timeRange: { from: dateTime(), to: dateTime(), raw: { from: 'now-1h', to: 'now' } }, } as unknown as GetExploreUrlArguments; it('should use raw range in explore url', async () => { - expect(getExploreUrl(args).then((data) => expect(data).toMatch(/from%22:%22now-1h%22,%22to%22:%22now/g))); + expect(await getExploreUrl(args)).toMatch(/from%22:%22now-1h%22,%22to%22:%22now/g); }); - it('should omit legendFormat in explore url', () => { - expect(getExploreUrl(args).then((data) => expect(data).not.toMatch(/legendFormat1/g))); - }); - it('should omit expression target in explore url', () => { - expect(getExploreUrl(args).then((data) => expect(data).not.toMatch(/__expr__/g))); + it('should omit expression target in explore url', async () => { + expect(await getExploreUrl(args)).not.toMatch(/__expr__/g); }); }); diff --git a/public/app/core/utils/explore.ts b/public/app/core/utils/explore.ts index 108530374f9..566003ad8d3 100644 --- a/public/app/core/utils/explore.ts +++ b/public/app/core/utils/explore.ts @@ -1,5 +1,4 @@ import { nanoid } from '@reduxjs/toolkit'; -import { omit } from 'lodash'; import { Unsubscribable } from 'rxjs'; import { v4 as uuidv4 } from 'uuid'; @@ -17,15 +16,15 @@ import { LogsSortOrder, rangeUtil, RawTimeRange, + ScopedVars, TimeRange, TimeZone, toURLRange, urlUtil, } from '@grafana/data'; -import { DataSourceSrv, getDataSourceSrv } from '@grafana/runtime'; +import { getDataSourceSrv } from '@grafana/runtime'; import { RefreshPicker } from '@grafana/ui'; import store from 'app/core/store'; -import { PanelModel } from 'app/features/dashboard/state'; import { ExpressionDatasourceUID } from 'app/features/expressions/types'; import { QueryOptions, QueryTransaction } from 'app/types/explore'; @@ -47,10 +46,10 @@ export const setLastUsedDatasourceUID = (orgId: number, datasourceUID: string) = store.setObject(lastUsedDatasourceKeyForOrgId(orgId), datasourceUID); export interface GetExploreUrlArguments { - panel: PanelModel; - /** Datasource service to query other datasources in case the panel datasource is mixed */ - datasourceSrv: DataSourceSrv; + queries: DataQuery[]; + dsRef: DataSourceRef | null | undefined; timeRange: TimeRange; + scopedVars: ScopedVars | undefined; } export function generateExploreId() { @@ -61,27 +60,23 @@ export function generateExploreId() { * Returns an Explore-URL that contains a panel's queries and the dashboard time range. */ export async function getExploreUrl(args: GetExploreUrlArguments): Promise { - const { panel, datasourceSrv, timeRange } = args; - let exploreDatasource = await datasourceSrv.get(panel.datasource); + const { queries, dsRef, timeRange, scopedVars } = args; + let exploreDatasource = await getDataSourceSrv().get(dsRef); - /** In Explore, we don't have legend formatter and we don't want to keep - * legend formatting as we can't change it - * - * We also don't have expressions, so filter those out + /* + * Explore does not support expressions so filter those out */ - let exploreTargets: DataQuery[] = panel.targets - .map((t) => omit(t, 'legendFormat')) - .filter((t) => t.datasource?.uid !== ExpressionDatasourceUID); + let exploreTargets: DataQuery[] = queries.filter((t) => t.datasource?.uid !== ExpressionDatasourceUID); + let url: string | undefined; if (exploreDatasource) { let state: Partial = { range: toURLRange(timeRange.raw) }; if (exploreDatasource.interpolateVariablesInQueries) { - const scopedVars = panel.scopedVars || {}; state = { ...state, datasource: exploreDatasource.uid, - queries: exploreDatasource.interpolateVariablesInQueries(exploreTargets, scopedVars), + queries: exploreDatasource.interpolateVariablesInQueries(exploreTargets, scopedVars ?? {}), }; } else { state = { diff --git a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx new file mode 100644 index 00000000000..30063fef945 --- /dev/null +++ b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx @@ -0,0 +1,98 @@ +import { getPanelPlugin } from '@grafana/data/test/__mocks__/pluginMocks'; +import { locationService } from '@grafana/runtime'; +import { SceneGridItem, SceneGridLayout, SceneQueryRunner, VizPanel, VizPanelMenu } from '@grafana/scenes'; +import { contextSrv } from 'app/core/services/context_srv'; +import { GetExploreUrlArguments } from 'app/core/utils/explore'; + +import { DashboardScene } from './DashboardScene'; +import { panelMenuBehavior } from './PanelMenuBehavior'; + +const mocks = { + contextSrv: jest.mocked(contextSrv), + getExploreUrl: jest.fn(), +}; + +jest.mock('app/core/utils/explore', () => ({ + ...jest.requireActual('app/core/utils/explore'), + getExploreUrl: (options: GetExploreUrlArguments) => { + return mocks.getExploreUrl(options); + }, +})); + +jest.mock('app/core/services/context_srv'); + +describe('panelMenuBehavior', () => { + beforeAll(() => { + locationService.push('/scenes/dashboard/dash-1?from=now-5m&to=now'); + }); + + it('Given standard panel', async () => { + const { menu, panel } = await buildTestScene({}); + + Object.assign(panel, 'getPlugin', () => getPanelPlugin({})); + + mocks.contextSrv.hasAccessToExplore.mockReturnValue(true); + mocks.getExploreUrl.mockReturnValue(Promise.resolve('/explore')); + + menu.activate(); + + await new Promise((r) => setTimeout(r, 1)); + + expect(menu.state.items?.length).toBe(4); + // verify view panel url keeps url params and adds viewPanel= + expect(menu.state.items?.[0].href).toBe('/scenes/dashboard/dash-1?from=now-5m&to=now&viewPanel=panel-12'); + // verify edit url keeps url time range + expect(menu.state.items?.[1].href).toBe('/scenes/dashboard/dash-1/panel-edit/12?from=now-5m&to=now'); + // verify explore url + expect(menu.state.items?.[2].href).toBe('/explore'); + + // Verify explore url is called with correct arguments + const getExploreArgs: GetExploreUrlArguments = mocks.getExploreUrl.mock.calls[0][0]; + expect(getExploreArgs.dsRef).toEqual({ uid: 'my-uid' }); + expect(getExploreArgs.queries).toEqual([{ query: 'buu', refId: 'A' }]); + expect(getExploreArgs.scopedVars?.__sceneObject?.value).toBe(panel); + + // verify inspect url keeps url params and adds inspect= + expect(menu.state.items?.[3].href).toBe('/scenes/dashboard/dash-1?from=now-5m&to=now&inspect=panel-12'); + }); +}); + +interface SceneOptions {} + +async function buildTestScene(options: SceneOptions) { + const menu = new VizPanelMenu({ + $behaviors: [panelMenuBehavior], + }); + + const panel = new VizPanel({ + title: 'Panel A', + pluginId: 'table', + key: 'panel-12', + menu, + $data: new SceneQueryRunner({ + datasource: { uid: 'my-uid' }, + queries: [{ query: 'buu', refId: 'A' }], + }), + }); + + const scene = new DashboardScene({ + title: 'hello', + uid: 'dash-1', + body: new SceneGridLayout({ + children: [ + new SceneGridItem({ + key: 'griditem-1', + x: 0, + y: 0, + width: 10, + height: 12, + body: panel, + }), + ], + }), + }); + + await new Promise((r) => setTimeout(r, 1)); + + return { scene, panel, menu }; +} diff --git a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx index e60df4ed57a..b5d95b0b307 100644 --- a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx +++ b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx @@ -1,9 +1,12 @@ import { locationUtil, PanelMenuItem } from '@grafana/data'; -import { locationService } from '@grafana/runtime'; -import { VizPanel, VizPanelMenu } from '@grafana/scenes'; +import { locationService, reportInteraction } from '@grafana/runtime'; +import { sceneGraph, VizPanel, VizPanelMenu } from '@grafana/scenes'; +import { contextSrv } from 'app/core/core'; import { t } from 'app/core/internationalization'; +import { getExploreUrl } from 'app/core/utils/explore'; +import { InspectTab } from 'app/features/inspector/types'; -import { getDashboardUrl, getPanelIdForVizPanel } from '../utils/utils'; +import { getDashboardUrl, getPanelIdForVizPanel, getQueryRunnerFor } from '../utils/utils'; import { DashboardScene } from './DashboardScene'; @@ -11,50 +14,68 @@ import { DashboardScene } from './DashboardScene'; * Behavior is called when VizPanelMenu is activated (ie when it's opened). */ export function panelMenuBehavior(menu: VizPanelMenu) { - // hm.. add another generic param to SceneObject to specify parent type? - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions - const panel = menu.parent as VizPanel; - const location = locationService.getLocation(); - const items: PanelMenuItem[] = []; - const panelId = getPanelIdForVizPanel(panel); - const dashboard = panel.getRoot(); + const asyncFunc = async () => { + // hm.. add another generic param to SceneObject to specify parent type? + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + const panel = menu.parent as VizPanel; + const location = locationService.getLocation(); + const items: PanelMenuItem[] = []; + const panelId = getPanelIdForVizPanel(panel); + const dashboard = panel.getRoot(); + const panelPlugin = panel.getPlugin(); + const queryRunner = getQueryRunnerFor(panel); - // TODO - // Add tracking via reportInteraction (but preserve the fact that these are normal links) + if (dashboard instanceof DashboardScene) { + items.push({ + text: t('panel.header-menu.view', `View`), + iconClassName: 'eye', + shortcut: 'v', + onClick: () => reportInteraction('dashboards_panelheader_menu', { item: 'view' }), + href: locationUtil.getUrlForPartial(location, { viewPanel: panel.state.key }), + }); + + // We could check isEditing here but I kind of think this should always be in the menu, + // and going into panel edit should make the dashboard go into edit mode is it's not already + items.push({ + text: t('panel.header-menu.edit', `Edit`), + iconClassName: 'eye', + shortcut: 'v', + onClick: () => reportInteraction('dashboards_panelheader_menu', { item: 'edit' }), + href: getDashboardUrl({ + uid: dashboard.state.uid, + subPath: `/panel-edit/${panelId}`, + currentQueryParams: location.search, + }), + }); + } + + if (contextSrv.hasAccessToExplore() && !panelPlugin?.meta.skipDataQuery && queryRunner) { + const timeRange = sceneGraph.getTimeRange(panel); + + items.push({ + text: t('panel.header-menu.explore', `Explore`), + iconClassName: 'compass', + shortcut: 'p x', + onClick: () => reportInteraction('dashboards_panelheader_menu', { item: 'explore' }), + href: await getExploreUrl({ + queries: queryRunner.state.queries, + dsRef: queryRunner.state.datasource, + timeRange: timeRange.state.value, + scopedVars: { __sceneObject: { value: panel } }, + }), + }); + } - if (dashboard instanceof DashboardScene) { items.push({ - text: t('panel.header-menu.view', `View`), - iconClassName: 'eye', - shortcut: 'v', - href: getDashboardUrl({ - uid: dashboard.state.uid, - currentQueryParams: location.search, - updateQuery: { filter: null, new: 'A' }, - }), + text: t('panel.header-menu.inspect', `Inspect`), + iconClassName: 'info-circle', + shortcut: 'i', + onClick: () => reportInteraction('dashboards_panelheader_menu', { item: 'inspect', tab: InspectTab.Data }), + href: locationUtil.getUrlForPartial(location, { inspect: panel.state.key }), }); - // We could check isEditing here but I kind of think this should always be in the menu, - // and going into panel edit should make the dashboard go into edit mode is it's not already - items.push({ - text: t('panel.header-menu.edit', `Edit`), - iconClassName: 'eye', - shortcut: 'v', - href: getDashboardUrl({ - uid: dashboard.state.uid, - subPath: `/panel-edit/${panelId}`, - currentQueryParams: location.search, - updateQuery: { filter: null, new: 'A' }, - }), - }); - } + menu.setState({ items }); + }; - items.push({ - text: t('panel.header-menu.inspect', `Inspect`), - iconClassName: 'info-circle', - shortcut: 'i', - href: locationUtil.getUrlForPartial(location, { inspect: panel.state.key }), - }); - - menu.setState({ items }); + asyncFunc(); } diff --git a/public/app/features/dashboard/utils/getPanelMenu.ts b/public/app/features/dashboard/utils/getPanelMenu.ts index bc4634c7d37..64e5d2eeff2 100644 --- a/public/app/features/dashboard/utils/getPanelMenu.ts +++ b/public/app/features/dashboard/utils/getPanelMenu.ts @@ -4,13 +4,7 @@ import { PluginExtensionPoints, type PluginExtensionPanelContext, } from '@grafana/data'; -import { - AngularComponent, - getDataSourceSrv, - locationService, - reportInteraction, - getPluginLinkExtensions, -} from '@grafana/runtime'; +import { AngularComponent, locationService, reportInteraction, getPluginLinkExtensions } from '@grafana/runtime'; import { PanelCtrl } from 'app/angular/panel/panel_ctrl'; import config from 'app/core/config'; import { t } from 'app/core/internationalization'; @@ -112,7 +106,6 @@ export function getPanelMenu( event.ctrlKey || event.metaKey ? (url: string) => window.open(`${config.appSubUrl}${url}`) : undefined; store.dispatch( navigateToExplore(panel, { - getDataSourceSrv, timeRange: getTimeSrv().timeRange(), getExploreUrl, openInNewWindow, diff --git a/public/app/features/explore/state/main.test.ts b/public/app/features/explore/state/main.test.ts index a0ab908725d..3a12dd5c66d 100644 --- a/public/app/features/explore/state/main.test.ts +++ b/public/app/features/explore/state/main.test.ts @@ -20,19 +20,17 @@ const getNavigateToExploreContext = async (openInNewWindow?: (url: string) => vo }; const datasource = new MockDataSourceApi(panel.datasource!.uid!); const get = jest.fn().mockResolvedValue(datasource); - const getDataSourceSrv = jest.fn().mockReturnValue({ get }); const getExploreUrl = jest.fn().mockResolvedValue(url); const timeRange = { from: dateTime(), to: dateTime() }; const dispatchedActions = await thunkTester({}) .givenThunk(navigateToExplore) - .whenThunkIsDispatched(panel, { getDataSourceSrv, timeRange, getExploreUrl, openInNewWindow }); + .whenThunkIsDispatched(panel, { timeRange, getExploreUrl, openInNewWindow }); return { url, panel, get, - getDataSourceSrv, timeRange, getExploreUrl, dispatchedActions, @@ -47,20 +45,14 @@ describe('navigateToExplore', () => { expect(locationService.getLocation().pathname).toEqual(url); }); - it('then getDataSourceSrv should have been once', async () => { - const { getDataSourceSrv } = await getNavigateToExploreContext(); - - expect(getDataSourceSrv).toHaveBeenCalledTimes(1); - }); - it('then getExploreUrl should have been called with correct arguments', async () => { - const { getExploreUrl, panel, getDataSourceSrv, timeRange } = await getNavigateToExploreContext(); + const { getExploreUrl, panel, timeRange } = await getNavigateToExploreContext(); expect(getExploreUrl).toHaveBeenCalledTimes(1); expect(getExploreUrl).toHaveBeenCalledWith({ - panel, - datasourceSrv: getDataSourceSrv(), + queries: panel.targets, timeRange, + dsRef: panel.datasource, }); }); }); @@ -73,22 +65,14 @@ describe('navigateToExplore', () => { expect(dispatchedActions).toEqual([]); }); - it('then getDataSourceSrv should have been once', async () => { - const { getDataSourceSrv } = await getNavigateToExploreContext(openInNewWindow); - - expect(getDataSourceSrv).toHaveBeenCalledTimes(1); - }); - it('then getExploreUrl should have been called with correct arguments', async () => { - const { getExploreUrl, panel, getDataSourceSrv, timeRange } = await getNavigateToExploreContext( - openInNewWindow - ); + const { getExploreUrl, panel, timeRange } = await getNavigateToExploreContext(openInNewWindow); expect(getExploreUrl).toHaveBeenCalledTimes(1); expect(getExploreUrl).toHaveBeenCalledWith({ - panel, - datasourceSrv: getDataSourceSrv(), + queries: panel.targets, timeRange, + dsRef: panel.datasource, }); }); diff --git a/public/app/features/explore/state/main.ts b/public/app/features/explore/state/main.ts index 7804fd7714e..46b8e90571e 100644 --- a/public/app/features/explore/state/main.ts +++ b/public/app/features/explore/state/main.ts @@ -2,7 +2,7 @@ import { createAction } from '@reduxjs/toolkit'; import { AnyAction } from 'redux'; import { SplitOpenOptions, TimeRange } from '@grafana/data'; -import { DataSourceSrv, locationService } from '@grafana/runtime'; +import { locationService } from '@grafana/runtime'; import { generateExploreId, GetExploreUrlArguments } from 'app/core/utils/explore'; import { PanelModel } from 'app/features/dashboard/state'; import { ExploreItemState, ExploreState } from 'app/types/explore'; @@ -105,7 +105,6 @@ const createNewSplitOpenPane = createAsyncThunk( ); export interface NavigateToExploreDependencies { - getDataSourceSrv: () => DataSourceSrv; timeRange: TimeRange; getExploreUrl: (args: GetExploreUrlArguments) => Promise; openInNewWindow?: (url: string) => void; @@ -116,11 +115,12 @@ export const navigateToExplore = ( dependencies: NavigateToExploreDependencies ): ThunkResult => { return async (dispatch) => { - const { getDataSourceSrv, timeRange, getExploreUrl, openInNewWindow } = dependencies; - const datasourceSrv = getDataSourceSrv(); + const { timeRange, getExploreUrl, openInNewWindow } = dependencies; + const path = await getExploreUrl({ - panel, - datasourceSrv, + queries: panel.targets, + dsRef: panel.datasource, + scopedVars: panel.scopedVars, timeRange, }); diff --git a/public/app/plugins/datasource/testdata/datasource.ts b/public/app/plugins/datasource/testdata/datasource.ts index 64752a08f27..ca72c786533 100644 --- a/public/app/plugins/datasource/testdata/datasource.ts +++ b/public/app/plugins/datasource/testdata/datasource.ts @@ -162,6 +162,11 @@ export class TestDataDataSource extends DataSourceWithBackend { } } + applyTemplateVariables(query: TestData, scopedVars: ScopedVars): TestData { + this.resolveTemplateVariables(query, scopedVars); + return query; + } + annotationDataTopicTest(target: TestData, req: DataQueryRequest): Observable { const events = this.buildFakeAnnotationEvents(req.range, target.lines ?? 10); const dataFrame = new ArrayDataFrame(events);