From bdfa127ee5c11948568d83579e60351792e65f4f Mon Sep 17 00:00:00 2001 From: Kristina Date: Mon, 21 Nov 2022 08:03:50 -0600 Subject: [PATCH] Explore: Notify when compact URL is used (#58684) * Add explore compact url notices * Add error checking around data links urls * Fix tests * remove global flag, add test for warning in title * Move feature tracking to initialization, add better error messaging * Fix test * Add compact url tests, fix styling bug, remove warning tooltip * Fix broken check, move tests to util file --- packages/grafana-data/src/types/explore.ts | 1 + .../components/DataLinks/DataLinkEditor.tsx | 7 ++++- .../DataLinksListItem.test.tsx | 11 +++++++ .../DataLinksListItem.tsx | 18 +++++++++-- .../grafana-ui/src/utils/dataLinks.test.ts | 31 +++++++++++++++++++ packages/grafana-ui/src/utils/dataLinks.ts | 5 +++ public/app/core/utils/explore.test.ts | 2 ++ public/app/core/utils/explore.ts | 5 +-- public/app/features/explore/Explore.test.tsx | 1 + public/app/features/explore/Explore.tsx | 18 ++++++++++- .../features/explore/ExplorePaneContainer.tsx | 20 ++++++++++-- .../app/features/explore/state/explorePane.ts | 9 ++++-- public/app/types/explore.ts | 2 ++ 13 files changed, 120 insertions(+), 10 deletions(-) create mode 100644 packages/grafana-ui/src/utils/dataLinks.test.ts diff --git a/packages/grafana-data/src/types/explore.ts b/packages/grafana-data/src/types/explore.ts index abc73743db0..ef3a44f7775 100644 --- a/packages/grafana-data/src/types/explore.ts +++ b/packages/grafana-data/src/types/explore.ts @@ -11,6 +11,7 @@ export interface ExploreUrlState { range: RawTimeRange; context?: string; panelsState?: ExplorePanelsState; + isFromCompactUrl?: boolean; } export interface ExplorePanelsState extends Partial> { diff --git a/packages/grafana-ui/src/components/DataLinks/DataLinkEditor.tsx b/packages/grafana-ui/src/components/DataLinks/DataLinkEditor.tsx index 820fae0b5bb..46bad70bd34 100644 --- a/packages/grafana-ui/src/components/DataLinks/DataLinkEditor.tsx +++ b/packages/grafana-ui/src/components/DataLinks/DataLinkEditor.tsx @@ -4,6 +4,7 @@ import React, { ChangeEvent } from 'react'; import { VariableSuggestion, GrafanaTheme2, DataLink } from '@grafana/data'; import { useStyles2 } from '../../themes/index'; +import { isCompactUrl } from '../../utils/dataLinks'; import { Field } from '../Forms/Field'; import { Input } from '../Input/Input'; import { Switch } from '../Switch/Switch'; @@ -50,7 +51,11 @@ export const DataLinkEditor: React.FC = React.memo( - + diff --git a/packages/grafana-ui/src/components/DataLinks/DataLinksInlineEditor/DataLinksListItem.test.tsx b/packages/grafana-ui/src/components/DataLinks/DataLinksInlineEditor/DataLinksListItem.test.tsx index 4e9d27d7f3d..b0877f8bcba 100644 --- a/packages/grafana-ui/src/components/DataLinks/DataLinksInlineEditor/DataLinksListItem.test.tsx +++ b/packages/grafana-ui/src/components/DataLinks/DataLinksInlineEditor/DataLinksListItem.test.tsx @@ -50,6 +50,17 @@ describe('DataLinksListItem', () => { expect(screen.getByText(/http:\/\/localhost\:3000/i)).toBeInTheDocument(); expect(screen.getByTitle(/http:\/\/localhost\:3000/i)).toBeInTheDocument(); }); + + it('that is a explore compact url, then the title should be a warning', () => { + const link = { + ...baseLink, + url: 'http://localhost:3000/explore?orgId=1&left=[%22now-1h%22,%22now%22,%22gdev-loki%22,{%22expr%22:%22{place=%22luna%22}%22,%22refId%22:%22A%22}]', + }; + setupTestContext({ link }); + + expect(screen.getByText(/http:\/\/localhost\:3000/i)).toBeInTheDocument(); + expect(screen.getByText(/Explore data link may not work in the future. Please edit./i)).toBeInTheDocument(); + }); }); describe('when link is missing title', () => { diff --git a/packages/grafana-ui/src/components/DataLinks/DataLinksInlineEditor/DataLinksListItem.tsx b/packages/grafana-ui/src/components/DataLinks/DataLinksInlineEditor/DataLinksListItem.tsx index 03fcfdd6623..2c9e721d332 100644 --- a/packages/grafana-ui/src/components/DataLinks/DataLinksInlineEditor/DataLinksListItem.tsx +++ b/packages/grafana-ui/src/components/DataLinks/DataLinksInlineEditor/DataLinksListItem.tsx @@ -4,6 +4,8 @@ import React, { FC } from 'react'; import { DataFrame, DataLink, GrafanaTheme2 } from '@grafana/data'; import { stylesFactory, useTheme2 } from '../../../themes'; +import { isCompactUrl } from '../../../utils/dataLinks'; +import { FieldValidationMessage } from '../../Forms/FieldValidationMessage'; import { IconButton } from '../../IconButton/IconButton'; import { HorizontalGroup, VerticalGroup } from '../../Layout/Layout'; @@ -25,11 +27,13 @@ export const DataLinksListItem: FC = ({ link, onEdit, on const hasTitle = title.trim() !== ''; const hasUrl = url.trim() !== ''; + const isCompactExploreUrl = isCompactUrl(url); + return (
-
+
{hasTitle ? title : 'Data link title not provided'}
@@ -37,9 +41,15 @@ export const DataLinksListItem: FC = ({ link, onEdit, on -
+
{hasUrl ? url : 'Data link url not provided'}
+ {isCompactExploreUrl && ( + Explore data link may not work in the future. Please edit. + )}
); @@ -54,6 +64,10 @@ const getDataLinkListItemStyles = stylesFactory((theme: GrafanaTheme2) => { margin-bottom: 0; } `, + errored: css` + color: ${theme.colors.error.text}; + font-style: italic; + `, notConfigured: css` font-style: italic; `, diff --git a/packages/grafana-ui/src/utils/dataLinks.test.ts b/packages/grafana-ui/src/utils/dataLinks.test.ts new file mode 100644 index 00000000000..a4f5ae1a61b --- /dev/null +++ b/packages/grafana-ui/src/utils/dataLinks.test.ts @@ -0,0 +1,31 @@ +import { isCompactUrl } from './dataLinks'; + +describe('Datalinks', () => { + it('isCompactUrl matches compact URL with segments', () => { + expect( + isCompactUrl( + 'http://localhost:3000/explore?orgId=1&left=[%22now-1h%22,%22now%22,%22gdev-loki%22,{%22expr%22:%22{place=%22luna%22}%22,%22refId%22:%22A%22}]' + ) + ).toEqual(true); + }); + + it('isCompactUrl matches compact URL without segments', () => { + expect(isCompactUrl('http://localhost:3000/explore?orgId=1&left=[%22now-1h%22,%22now%22,%22gdev-loki%22]')).toEqual( + true + ); + }); + + it('isCompactUrl matches compact URL with right pane', () => { + expect( + isCompactUrl('http://localhost:3000/explore?orgId=1&right=[%22now-1h%22,%22now%22,%22gdev-loki%22]') + ).toEqual(true); + }); + + it('isCompactUrl does not match non-compact url', () => { + expect( + isCompactUrl( + 'http://localhost:3000/explore?orgId=1&left={"datasource":"test[datasource]","queries":[{"refId":"A","datasource":{"type":"prometheus","uid":"gdev-prometheus"}}],"range":{"from":"now-1h","to":"now"}}' + ) + ).toEqual(false); + }); +}); diff --git a/packages/grafana-ui/src/utils/dataLinks.ts b/packages/grafana-ui/src/utils/dataLinks.ts index 5d2a6db23fd..e5111f90851 100644 --- a/packages/grafana-ui/src/utils/dataLinks.ts +++ b/packages/grafana-ui/src/utils/dataLinks.ts @@ -18,3 +18,8 @@ export const linkModelToContextMenuItems: (links: () => LinkModel[]) => MenuItem }; }); }; + +export const isCompactUrl = (url: string) => { + const compactExploreUrlRegex = /\/explore\?.*&(left|right)=\[(.*\,){2,}(.*){1}\]/; + return compactExploreUrlRegex.test(url); +}; diff --git a/public/app/core/utils/explore.test.ts b/public/app/core/utils/explore.test.ts index 63e99705130..0410a47f1d3 100644 --- a/public/app/core/utils/explore.test.ts +++ b/public/app/core/utils/explore.test.ts @@ -122,6 +122,7 @@ describe('state functions', () => { const state = { ...DEFAULT_EXPLORE_STATE, datasource: 'foo', + isFromCompactUrl: false, queries: [ { expr: 'metric{test="a/b"}', @@ -146,6 +147,7 @@ describe('state functions', () => { const state = { ...DEFAULT_EXPLORE_STATE, datasource: 'foo', + isFromCompactUrl: false, queries: [ { expr: 'metric{test="a/b"}', diff --git a/public/app/core/utils/explore.ts b/public/app/core/utils/explore.ts index dce6c2e66ad..2237ee3eb56 100644 --- a/public/app/core/utils/explore.ts +++ b/public/app/core/utils/explore.ts @@ -219,7 +219,8 @@ export function parseUrlState(initial: string | undefined): ExploreUrlState { } if (!Array.isArray(parsed)) { - return parsed; + const urlState = { ...parsed, isFromCompactUrl: false }; + return urlState; } if (parsed.length <= ParseUrlStateIndex.SegmentsStart) { @@ -236,7 +237,7 @@ export function parseUrlState(initial: string | undefined): ExploreUrlState { const queries = parsedSegments.filter((segment) => !isSegment(segment, 'ui', 'mode', '__panelsState')); const panelsState = parsedSegments.find((segment) => isSegment(segment, '__panelsState'))?.__panelsState; - return { datasource, queries, range, panelsState }; + return { datasource, queries, range, panelsState, isFromCompactUrl: true }; } export function generateKey(index = 0): string { diff --git a/public/app/features/explore/Explore.test.tsx b/public/app/features/explore/Explore.test.tsx index 619ef022261..db4aec6269a 100644 --- a/public/app/features/explore/Explore.test.tsx +++ b/public/app/features/explore/Explore.test.tsx @@ -84,6 +84,7 @@ const dummyProps: Props = { showFlameGraph: true, splitOpen: (() => {}) as any, splitted: false, + isFromCompactUrl: false, eventBus: new EventBusSrv(), }; diff --git a/public/app/features/explore/Explore.tsx b/public/app/features/explore/Explore.tsx index e4b3f488fac..128266630d5 100644 --- a/public/app/features/explore/Explore.tsx +++ b/public/app/features/explore/Explore.tsx @@ -18,9 +18,10 @@ import { } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { config, getDataSourceSrv, reportInteraction } from '@grafana/runtime'; -import { CustomScrollbar, ErrorBoundaryAlert, Themeable2, withTheme2, PanelContainer } from '@grafana/ui'; +import { CustomScrollbar, ErrorBoundaryAlert, Themeable2, withTheme2, PanelContainer, Alert } from '@grafana/ui'; import { FILTER_FOR_OPERATOR, FILTER_OUT_OPERATOR, FilterItem } from '@grafana/ui/src/components/Table/types'; import appEvents from 'app/core/app_events'; +import { FadeIn } from 'app/core/components/Animations/FadeIn'; import { supportedFeatures } from 'app/core/history/richHistoryStorageProvider'; import { MIXED_DATASOURCE_NAME } from 'app/plugins/datasource/mixed/MixedDataSource'; import { getNodeGraphDataFrames } from 'app/plugins/panel/nodeGraph/utils'; @@ -270,6 +271,17 @@ export class Explore extends React.PureComponent { return ; } + renderCompactUrlWarning() { + return ( + + + The URL that brought you here was a compact URL - this format will soon be deprecated. Please replace the URL + previously saved with the URL available now. + + + ); + } + renderGraphPanel(width: number) { const { graphResult, absoluteRange, timeZone, queryResponse, loading, showFlameGraph } = this.props; @@ -382,6 +394,7 @@ export class Explore extends React.PureComponent { showFlameGraph, splitted, timeZone, + isFromCompactUrl, } = this.props; const { openDrawer } = this.state; const styles = getStyles(theme); @@ -407,6 +420,7 @@ export class Explore extends React.PureComponent { scrollRefCallback={(scrollElement) => (this.scrollElement = scrollElement || undefined)} > + {isFromCompactUrl ? this.renderCompactUrlWarning() : null} {datasourceMissing ? this.renderEmptyState(styles.exploreContainer) : null} {datasourceInstance && (
{ } async componentDidMount() { - const { initialized, exploreId, initialDatasource, initialQueries, initialRange, panelsState, orgId } = this.props; + const { + initialized, + exploreId, + initialDatasource, + initialQueries, + initialRange, + panelsState, + orgId, + isFromCompactUrl, + } = this.props; const width = this.el?.offsetWidth ?? 0; // initialize the whole explore first time we mount and if browser history contains a change in datasource if (!initialized) { @@ -108,6 +118,10 @@ class ExplorePaneContainerUnconnected extends React.PureComponent { } } + if (isFromCompactUrl) { + reportInteraction('grafana_explore_compact_notice'); + } + this.props.initializeExplore( exploreId, rootDatasourceOverride || queries[0]?.datasource || initialDatasource, @@ -115,7 +129,8 @@ class ExplorePaneContainerUnconnected extends React.PureComponent { initialRange, width, this.exploreEvents, - panelsState + panelsState, + isFromCompactUrl ); } } @@ -172,6 +187,7 @@ function mapStateToProps(state: StoreState, props: OwnProps) { initialRange, panelsState, orgId: state.user.orgId, + isFromCompactUrl: urlState.isFromCompactUrl || false, }; } diff --git a/public/app/features/explore/state/explorePane.ts b/public/app/features/explore/state/explorePane.ts index ecbfa669b7f..20cff532e12 100644 --- a/public/app/features/explore/state/explorePane.ts +++ b/public/app/features/explore/state/explorePane.ts @@ -98,6 +98,7 @@ export interface InitializeExplorePayload { range: TimeRange; history: HistoryItem[]; datasourceInstance?: DataSourceApi; + isFromCompactUrl?: boolean; } export const initializeExploreAction = createAction('explore/initializeExplore'); @@ -132,7 +133,8 @@ export function initializeExplore( range: TimeRange, containerWidth: number, eventBridge: EventBusExtended, - panelsState?: ExplorePanelsState + panelsState?: ExplorePanelsState, + isFromCompactUrl?: boolean ): ThunkResult { return async (dispatch, getState) => { const exploreDatasources = getDataSourceSrv().getList(); @@ -155,6 +157,7 @@ export function initializeExplore( range, datasourceInstance: instance, history, + isFromCompactUrl, }) ); if (panelsState !== undefined) { @@ -272,7 +275,8 @@ export const paneReducer = (state: ExploreItemState = makeExplorePaneState(), ac } if (initializeExploreAction.match(action)) { - const { containerWidth, eventBridge, queries, range, datasourceInstance, history } = action.payload; + const { containerWidth, eventBridge, queries, range, datasourceInstance, history, isFromCompactUrl } = + action.payload; return { ...state, @@ -287,6 +291,7 @@ export const paneReducer = (state: ExploreItemState = makeExplorePaneState(), ac datasourceMissing: !datasourceInstance, queryResponse: createEmptyQueryResponse(), cache: [], + isFromCompactUrl: isFromCompactUrl || false, }; } diff --git a/public/app/types/explore.ts b/public/app/types/explore.ts index 7891519890a..c0936606eb6 100644 --- a/public/app/types/explore.ts +++ b/public/app/types/explore.ts @@ -203,6 +203,8 @@ export interface ExploreItemState { logsVolumeData?: DataQueryResponse; panelsState: ExplorePanelsState; + + isFromCompactUrl?: boolean; } export interface ExploreUpdateState {