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
This commit is contained in:
Kristina
2022-11-21 08:03:50 -06:00
committed by GitHub
parent b1ffe18599
commit bdfa127ee5
13 changed files with 120 additions and 10 deletions
@@ -11,6 +11,7 @@ export interface ExploreUrlState<T extends DataQuery = AnyQuery> {
range: RawTimeRange; range: RawTimeRange;
context?: string; context?: string;
panelsState?: ExplorePanelsState; panelsState?: ExplorePanelsState;
isFromCompactUrl?: boolean;
} }
export interface ExplorePanelsState extends Partial<Record<PreferredVisualisationType, {}>> { export interface ExplorePanelsState extends Partial<Record<PreferredVisualisationType, {}>> {
@@ -4,6 +4,7 @@ import React, { ChangeEvent } from 'react';
import { VariableSuggestion, GrafanaTheme2, DataLink } from '@grafana/data'; import { VariableSuggestion, GrafanaTheme2, DataLink } from '@grafana/data';
import { useStyles2 } from '../../themes/index'; import { useStyles2 } from '../../themes/index';
import { isCompactUrl } from '../../utils/dataLinks';
import { Field } from '../Forms/Field'; import { Field } from '../Forms/Field';
import { Input } from '../Input/Input'; import { Input } from '../Input/Input';
import { Switch } from '../Switch/Switch'; import { Switch } from '../Switch/Switch';
@@ -50,7 +51,11 @@ export const DataLinkEditor: React.FC<DataLinkEditorProps> = React.memo(
<Input value={value.title} onChange={onTitleChange} placeholder="Show details" /> <Input value={value.title} onChange={onTitleChange} placeholder="Show details" />
</Field> </Field>
<Field label="URL"> <Field
label="URL"
invalid={isCompactUrl(value.url)}
error="Data link is an Explore URL in a deprecated format. Please visit the URL to be redirected, and edit this data link to use that URL."
>
<DataLinkInput value={value.url} onChange={onUrlChange} suggestions={suggestions} /> <DataLinkInput value={value.url} onChange={onUrlChange} suggestions={suggestions} />
</Field> </Field>
@@ -50,6 +50,17 @@ describe('DataLinksListItem', () => {
expect(screen.getByText(/http:\/\/localhost\:3000/i)).toBeInTheDocument(); expect(screen.getByText(/http:\/\/localhost\:3000/i)).toBeInTheDocument();
expect(screen.getByTitle(/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', () => { describe('when link is missing title', () => {
@@ -4,6 +4,8 @@ import React, { FC } from 'react';
import { DataFrame, DataLink, GrafanaTheme2 } from '@grafana/data'; import { DataFrame, DataLink, GrafanaTheme2 } from '@grafana/data';
import { stylesFactory, useTheme2 } from '../../../themes'; import { stylesFactory, useTheme2 } from '../../../themes';
import { isCompactUrl } from '../../../utils/dataLinks';
import { FieldValidationMessage } from '../../Forms/FieldValidationMessage';
import { IconButton } from '../../IconButton/IconButton'; import { IconButton } from '../../IconButton/IconButton';
import { HorizontalGroup, VerticalGroup } from '../../Layout/Layout'; import { HorizontalGroup, VerticalGroup } from '../../Layout/Layout';
@@ -25,11 +27,13 @@ export const DataLinksListItem: FC<DataLinksListItemProps> = ({ link, onEdit, on
const hasTitle = title.trim() !== ''; const hasTitle = title.trim() !== '';
const hasUrl = url.trim() !== ''; const hasUrl = url.trim() !== '';
const isCompactExploreUrl = isCompactUrl(url);
return ( return (
<div className={styles.wrapper}> <div className={styles.wrapper}>
<VerticalGroup spacing="xs"> <VerticalGroup spacing="xs">
<HorizontalGroup justify="space-between" align="flex-start" width="100%"> <HorizontalGroup justify="space-between" align="flex-start" width="100%">
<div className={cx(styles.title, !hasTitle && styles.notConfigured)}> <div className={cx(styles.url, !hasUrl && styles.notConfigured, isCompactExploreUrl && styles.errored)}>
{hasTitle ? title : 'Data link title not provided'} {hasTitle ? title : 'Data link title not provided'}
</div> </div>
<HorizontalGroup> <HorizontalGroup>
@@ -37,9 +41,15 @@ export const DataLinksListItem: FC<DataLinksListItemProps> = ({ link, onEdit, on
<IconButton name="times" onClick={onRemove} /> <IconButton name="times" onClick={onRemove} />
</HorizontalGroup> </HorizontalGroup>
</HorizontalGroup> </HorizontalGroup>
<div className={cx(styles.url, !hasUrl && styles.notConfigured)} title={url}> <div
className={cx(styles.url, !hasUrl && styles.notConfigured, isCompactExploreUrl && styles.errored)}
title={url}
>
{hasUrl ? url : 'Data link url not provided'} {hasUrl ? url : 'Data link url not provided'}
</div> </div>
{isCompactExploreUrl && (
<FieldValidationMessage>Explore data link may not work in the future. Please edit.</FieldValidationMessage>
)}
</VerticalGroup> </VerticalGroup>
</div> </div>
); );
@@ -54,6 +64,10 @@ const getDataLinkListItemStyles = stylesFactory((theme: GrafanaTheme2) => {
margin-bottom: 0; margin-bottom: 0;
} }
`, `,
errored: css`
color: ${theme.colors.error.text};
font-style: italic;
`,
notConfigured: css` notConfigured: css`
font-style: italic; font-style: italic;
`, `,
@@ -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);
});
});
@@ -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);
};
+2
View File
@@ -122,6 +122,7 @@ describe('state functions', () => {
const state = { const state = {
...DEFAULT_EXPLORE_STATE, ...DEFAULT_EXPLORE_STATE,
datasource: 'foo', datasource: 'foo',
isFromCompactUrl: false,
queries: [ queries: [
{ {
expr: 'metric{test="a/b"}', expr: 'metric{test="a/b"}',
@@ -146,6 +147,7 @@ describe('state functions', () => {
const state = { const state = {
...DEFAULT_EXPLORE_STATE, ...DEFAULT_EXPLORE_STATE,
datasource: 'foo', datasource: 'foo',
isFromCompactUrl: false,
queries: [ queries: [
{ {
expr: 'metric{test="a/b"}', expr: 'metric{test="a/b"}',
+3 -2
View File
@@ -219,7 +219,8 @@ export function parseUrlState(initial: string | undefined): ExploreUrlState {
} }
if (!Array.isArray(parsed)) { if (!Array.isArray(parsed)) {
return parsed; const urlState = { ...parsed, isFromCompactUrl: false };
return urlState;
} }
if (parsed.length <= ParseUrlStateIndex.SegmentsStart) { 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 queries = parsedSegments.filter((segment) => !isSegment(segment, 'ui', 'mode', '__panelsState'));
const panelsState = parsedSegments.find((segment) => isSegment(segment, '__panelsState'))?.__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 { export function generateKey(index = 0): string {
@@ -84,6 +84,7 @@ const dummyProps: Props = {
showFlameGraph: true, showFlameGraph: true,
splitOpen: (() => {}) as any, splitOpen: (() => {}) as any,
splitted: false, splitted: false,
isFromCompactUrl: false,
eventBus: new EventBusSrv(), eventBus: new EventBusSrv(),
}; };
+17 -1
View File
@@ -18,9 +18,10 @@ import {
} from '@grafana/data'; } from '@grafana/data';
import { selectors } from '@grafana/e2e-selectors'; import { selectors } from '@grafana/e2e-selectors';
import { config, getDataSourceSrv, reportInteraction } from '@grafana/runtime'; 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 { FILTER_FOR_OPERATOR, FILTER_OUT_OPERATOR, FilterItem } from '@grafana/ui/src/components/Table/types';
import appEvents from 'app/core/app_events'; import appEvents from 'app/core/app_events';
import { FadeIn } from 'app/core/components/Animations/FadeIn';
import { supportedFeatures } from 'app/core/history/richHistoryStorageProvider'; import { supportedFeatures } from 'app/core/history/richHistoryStorageProvider';
import { MIXED_DATASOURCE_NAME } from 'app/plugins/datasource/mixed/MixedDataSource'; import { MIXED_DATASOURCE_NAME } from 'app/plugins/datasource/mixed/MixedDataSource';
import { getNodeGraphDataFrames } from 'app/plugins/panel/nodeGraph/utils'; import { getNodeGraphDataFrames } from 'app/plugins/panel/nodeGraph/utils';
@@ -270,6 +271,17 @@ export class Explore extends React.PureComponent<Props, ExploreState> {
return <NoData />; return <NoData />;
} }
renderCompactUrlWarning() {
return (
<FadeIn in={true} duration={100}>
<Alert severity="warning" title="Compact URL Deprecation Notice" topSpacing={2}>
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.
</Alert>
</FadeIn>
);
}
renderGraphPanel(width: number) { renderGraphPanel(width: number) {
const { graphResult, absoluteRange, timeZone, queryResponse, loading, showFlameGraph } = this.props; const { graphResult, absoluteRange, timeZone, queryResponse, loading, showFlameGraph } = this.props;
@@ -382,6 +394,7 @@ export class Explore extends React.PureComponent<Props, ExploreState> {
showFlameGraph, showFlameGraph,
splitted, splitted,
timeZone, timeZone,
isFromCompactUrl,
} = this.props; } = this.props;
const { openDrawer } = this.state; const { openDrawer } = this.state;
const styles = getStyles(theme); const styles = getStyles(theme);
@@ -407,6 +420,7 @@ export class Explore extends React.PureComponent<Props, ExploreState> {
scrollRefCallback={(scrollElement) => (this.scrollElement = scrollElement || undefined)} scrollRefCallback={(scrollElement) => (this.scrollElement = scrollElement || undefined)}
> >
<ExploreToolbar exploreId={exploreId} onChangeTime={this.onChangeTime} topOfViewRef={this.topOfViewRef} /> <ExploreToolbar exploreId={exploreId} onChangeTime={this.onChangeTime} topOfViewRef={this.topOfViewRef} />
{isFromCompactUrl ? this.renderCompactUrlWarning() : null}
{datasourceMissing ? this.renderEmptyState(styles.exploreContainer) : null} {datasourceMissing ? this.renderEmptyState(styles.exploreContainer) : null}
{datasourceInstance && ( {datasourceInstance && (
<div <div
@@ -503,6 +517,7 @@ function mapStateToProps(state: StoreState, { exploreId }: ExploreProps) {
showNodeGraph, showNodeGraph,
showFlameGraph, showFlameGraph,
loading, loading,
isFromCompactUrl,
} = item; } = item;
return { return {
@@ -525,6 +540,7 @@ function mapStateToProps(state: StoreState, { exploreId }: ExploreProps) {
showFlameGraph, showFlameGraph,
splitted: isSplit(state), splitted: isSplit(state),
loading, loading,
isFromCompactUrl: isFromCompactUrl || false,
}; };
} }
@@ -5,6 +5,7 @@ import { connect, ConnectedProps } from 'react-redux';
import { ExploreUrlState, EventBusExtended, EventBusSrv, GrafanaTheme2, EventBus } from '@grafana/data'; import { ExploreUrlState, EventBusExtended, EventBusSrv, GrafanaTheme2, EventBus } from '@grafana/data';
import { selectors } from '@grafana/e2e-selectors'; import { selectors } from '@grafana/e2e-selectors';
import { reportInteraction } from '@grafana/runtime';
import { Themeable2, withTheme2 } from '@grafana/ui'; import { Themeable2, withTheme2 } from '@grafana/ui';
import { config } from 'app/core/config'; import { config } from 'app/core/config';
import store from 'app/core/store'; import store from 'app/core/store';
@@ -70,7 +71,16 @@ class ExplorePaneContainerUnconnected extends React.PureComponent<Props> {
} }
async componentDidMount() { 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; const width = this.el?.offsetWidth ?? 0;
// initialize the whole explore first time we mount and if browser history contains a change in datasource // initialize the whole explore first time we mount and if browser history contains a change in datasource
if (!initialized) { if (!initialized) {
@@ -108,6 +118,10 @@ class ExplorePaneContainerUnconnected extends React.PureComponent<Props> {
} }
} }
if (isFromCompactUrl) {
reportInteraction('grafana_explore_compact_notice');
}
this.props.initializeExplore( this.props.initializeExplore(
exploreId, exploreId,
rootDatasourceOverride || queries[0]?.datasource || initialDatasource, rootDatasourceOverride || queries[0]?.datasource || initialDatasource,
@@ -115,7 +129,8 @@ class ExplorePaneContainerUnconnected extends React.PureComponent<Props> {
initialRange, initialRange,
width, width,
this.exploreEvents, this.exploreEvents,
panelsState panelsState,
isFromCompactUrl
); );
} }
} }
@@ -172,6 +187,7 @@ function mapStateToProps(state: StoreState, props: OwnProps) {
initialRange, initialRange,
panelsState, panelsState,
orgId: state.user.orgId, orgId: state.user.orgId,
isFromCompactUrl: urlState.isFromCompactUrl || false,
}; };
} }
@@ -98,6 +98,7 @@ export interface InitializeExplorePayload {
range: TimeRange; range: TimeRange;
history: HistoryItem[]; history: HistoryItem[];
datasourceInstance?: DataSourceApi; datasourceInstance?: DataSourceApi;
isFromCompactUrl?: boolean;
} }
export const initializeExploreAction = createAction<InitializeExplorePayload>('explore/initializeExplore'); export const initializeExploreAction = createAction<InitializeExplorePayload>('explore/initializeExplore');
@@ -132,7 +133,8 @@ export function initializeExplore(
range: TimeRange, range: TimeRange,
containerWidth: number, containerWidth: number,
eventBridge: EventBusExtended, eventBridge: EventBusExtended,
panelsState?: ExplorePanelsState panelsState?: ExplorePanelsState,
isFromCompactUrl?: boolean
): ThunkResult<void> { ): ThunkResult<void> {
return async (dispatch, getState) => { return async (dispatch, getState) => {
const exploreDatasources = getDataSourceSrv().getList(); const exploreDatasources = getDataSourceSrv().getList();
@@ -155,6 +157,7 @@ export function initializeExplore(
range, range,
datasourceInstance: instance, datasourceInstance: instance,
history, history,
isFromCompactUrl,
}) })
); );
if (panelsState !== undefined) { if (panelsState !== undefined) {
@@ -272,7 +275,8 @@ export const paneReducer = (state: ExploreItemState = makeExplorePaneState(), ac
} }
if (initializeExploreAction.match(action)) { if (initializeExploreAction.match(action)) {
const { containerWidth, eventBridge, queries, range, datasourceInstance, history } = action.payload; const { containerWidth, eventBridge, queries, range, datasourceInstance, history, isFromCompactUrl } =
action.payload;
return { return {
...state, ...state,
@@ -287,6 +291,7 @@ export const paneReducer = (state: ExploreItemState = makeExplorePaneState(), ac
datasourceMissing: !datasourceInstance, datasourceMissing: !datasourceInstance,
queryResponse: createEmptyQueryResponse(), queryResponse: createEmptyQueryResponse(),
cache: [], cache: [],
isFromCompactUrl: isFromCompactUrl || false,
}; };
} }
+2
View File
@@ -203,6 +203,8 @@ export interface ExploreItemState {
logsVolumeData?: DataQueryResponse; logsVolumeData?: DataQueryResponse;
panelsState: ExplorePanelsState; panelsState: ExplorePanelsState;
isFromCompactUrl?: boolean;
} }
export interface ExploreUpdateState { export interface ExploreUpdateState {