Commit db85c3e7 by Jess Committed by GitHub

Rich history UX fixes (#22783)

* Initial commit

* Visualised renamed or deleted  datasources as well, if they have queries

* Pass ds image to card and information if the datasource was removed/renamed

* Set up card with datasource info and change run query

* Style comment, run button

* Fix button naming

* Remember last filters

* Update public/app/core/store.ts

* Update public/app/features/explore/RichHistory/RichHistory.tsx

* Update comments

* Rename datasource to data source

* Add test coverage, fix naming

* Remove unused styles, add feedback info

Co-authored-by: Ivana <ivana.huckova@gmail.com>
Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com>
parent c7951a25
...@@ -7,6 +7,7 @@ import { ...@@ -7,6 +7,7 @@ import {
createQueryHeading, createQueryHeading,
createDataQuery, createDataQuery,
deleteAllFromRichHistory, deleteAllFromRichHistory,
deleteQueryInRichHistory,
} from './richHistory'; } from './richHistory';
import store from 'app/core/store'; import store from 'app/core/store';
import { SortOrder } from './explore'; import { SortOrder } from './explore';
...@@ -135,6 +136,18 @@ describe('updateCommentInRichHistory', () => { ...@@ -135,6 +136,18 @@ describe('updateCommentInRichHistory', () => {
}); });
}); });
describe('deleteQueryInRichHistory', () => {
it('should delete query in query in history', () => {
const deletedHistory = deleteQueryInRichHistory(mock.history, 1);
expect(deletedHistory).toEqual([]);
});
it('should delete query in localStorage', () => {
deleteQueryInRichHistory(mock.history, 1);
expect(store.exists(key)).toBeTruthy();
expect(store.getObject(key)).toEqual([]);
});
});
describe('mapNumbertoTimeInSlider', () => { describe('mapNumbertoTimeInSlider', () => {
it('should correctly map number to value', () => { it('should correctly map number to value', () => {
const value = mapNumbertoTimeInSlider(25); const value = mapNumbertoTimeInSlider(25);
......
...@@ -6,6 +6,7 @@ import { DataQuery, ExploreMode } from '@grafana/data'; ...@@ -6,6 +6,7 @@ import { DataQuery, ExploreMode } from '@grafana/data';
import { renderUrl } from 'app/core/utils/url'; import { renderUrl } from 'app/core/utils/url';
import store from 'app/core/store'; import store from 'app/core/store';
import { serializeStateToUrlParam, SortOrder } from './explore'; import { serializeStateToUrlParam, SortOrder } from './explore';
import { getExploreDatasources } from '../../features/explore/state/selectors';
// Types // Types
import { ExploreUrlState, RichHistoryQuery } from 'app/types/explore'; import { ExploreUrlState, RichHistoryQuery } from 'app/types/explore';
...@@ -16,6 +17,7 @@ export const RICH_HISTORY_SETTING_KEYS = { ...@@ -16,6 +17,7 @@ export const RICH_HISTORY_SETTING_KEYS = {
retentionPeriod: 'grafana.explore.richHistory.retentionPeriod', retentionPeriod: 'grafana.explore.richHistory.retentionPeriod',
starredTabAsFirstTab: 'grafana.explore.richHistory.starredTabAsFirstTab', starredTabAsFirstTab: 'grafana.explore.richHistory.starredTabAsFirstTab',
activeDatasourceOnly: 'grafana.explore.richHistory.activeDatasourceOnly', activeDatasourceOnly: 'grafana.explore.richHistory.activeDatasourceOnly',
datasourceFilters: 'grafana.explore.richHistory.datasourceFilters',
}; };
/* /*
...@@ -113,6 +115,12 @@ export function updateCommentInRichHistory( ...@@ -113,6 +115,12 @@ export function updateCommentInRichHistory(
return updatedQueries; return updatedQueries;
} }
export function deleteQueryInRichHistory(richHistory: RichHistoryQuery[], ts: number) {
const updatedQueries = richHistory.filter(query => query.ts !== ts);
store.setObject(RICH_HISTORY_KEY, updatedQueries);
return updatedQueries;
}
export const sortQueries = (array: RichHistoryQuery[], sortOrder: SortOrder) => { export const sortQueries = (array: RichHistoryQuery[], sortOrder: SortOrder) => {
let sortFunc; let sortFunc;
...@@ -257,3 +265,31 @@ export function mapQueriesToHeadings(query: RichHistoryQuery[], sortOrder: SortO ...@@ -257,3 +265,31 @@ export function mapQueriesToHeadings(query: RichHistoryQuery[], sortOrder: SortO
return mappedQueriesToHeadings; return mappedQueriesToHeadings;
} }
/* Create datasource list with images. If specific datasource retrieved from Rich history is not part of
* exploreDatasources add generic datasource image and add property isRemoved = true.
*/
export function createDatasourcesList(queriesDatasources: string[]) {
const exploreDatasources = getExploreDatasources();
const datasources: Array<{ label: string; value: string; imgUrl: string; isRemoved: boolean }> = [];
queriesDatasources.forEach(queryDsName => {
const index = exploreDatasources.findIndex(exploreDs => exploreDs.name === queryDsName);
if (index !== -1) {
datasources.push({
label: queryDsName,
value: queryDsName,
imgUrl: exploreDatasources[index].meta.info.logos.small,
isRemoved: false,
});
} else {
datasources.push({
label: queryDsName,
value: queryDsName,
imgUrl: 'public/img/icn-datasource.svg',
isRemoved: true,
});
}
});
return datasources;
}
...@@ -83,8 +83,8 @@ class UnThemedRichHistory extends PureComponent<RichHistoryProps, RichHistorySta ...@@ -83,8 +83,8 @@ class UnThemedRichHistory extends PureComponent<RichHistoryProps, RichHistorySta
super(props); super(props);
this.state = { this.state = {
activeTab: this.props.firstTab, activeTab: this.props.firstTab,
datasourceFilters: null,
sortOrder: SortOrder.Descending, sortOrder: SortOrder.Descending,
datasourceFilters: store.getObject(RICH_HISTORY_SETTING_KEYS.datasourceFilters, null),
retentionPeriod: store.getObject(RICH_HISTORY_SETTING_KEYS.retentionPeriod, 7), retentionPeriod: store.getObject(RICH_HISTORY_SETTING_KEYS.retentionPeriod, 7),
starredTabAsFirstTab: store.getBool(RICH_HISTORY_SETTING_KEYS.starredTabAsFirstTab, false), starredTabAsFirstTab: store.getBool(RICH_HISTORY_SETTING_KEYS.starredTabAsFirstTab, false),
activeDatasourceOnly: store.getBool(RICH_HISTORY_SETTING_KEYS.activeDatasourceOnly, false), activeDatasourceOnly: store.getBool(RICH_HISTORY_SETTING_KEYS.activeDatasourceOnly, false),
...@@ -115,6 +115,7 @@ class UnThemedRichHistory extends PureComponent<RichHistoryProps, RichHistorySta ...@@ -115,6 +115,7 @@ class UnThemedRichHistory extends PureComponent<RichHistoryProps, RichHistorySta
}; };
onSelectDatasourceFilters = (value: SelectableValue[] | null) => { onSelectDatasourceFilters = (value: SelectableValue[] | null) => {
store.setObject(RICH_HISTORY_SETTING_KEYS.datasourceFilters, value);
this.setState({ datasourceFilters: value }); this.setState({ datasourceFilters: value });
}; };
...@@ -133,7 +134,7 @@ class UnThemedRichHistory extends PureComponent<RichHistoryProps, RichHistorySta ...@@ -133,7 +134,7 @@ class UnThemedRichHistory extends PureComponent<RichHistoryProps, RichHistorySta
? this.onSelectDatasourceFilters([ ? this.onSelectDatasourceFilters([
{ label: this.props.activeDatasourceInstance, value: this.props.activeDatasourceInstance }, { label: this.props.activeDatasourceInstance, value: this.props.activeDatasourceInstance },
]) ])
: this.onSelectDatasourceFilters(null); : this.onSelectDatasourceFilters(this.state.datasourceFilters);
} }
componentDidMount() { componentDidMount() {
......
...@@ -15,10 +15,11 @@ const setup = (propOverrides?: Partial<Props>) => { ...@@ -15,10 +15,11 @@ const setup = (propOverrides?: Partial<Props>) => {
queries: ['query1', 'query2', 'query3'], queries: ['query1', 'query2', 'query3'],
sessionName: '', sessionName: '',
}, },
changeQuery: jest.fn(), dsImg: '/app/img',
isRemoved: false,
changeDatasource: jest.fn(), changeDatasource: jest.fn(),
clearQueries: jest.fn(),
updateRichHistory: jest.fn(), updateRichHistory: jest.fn(),
setQueries: jest.fn(),
exploreId: ExploreId.left, exploreId: ExploreId.left,
datasourceInstance: { name: 'Datasource' } as DataSourceApi, datasourceInstance: { name: 'Datasource' } as DataSourceApi,
}; };
...@@ -62,6 +63,18 @@ describe('RichHistoryCard', () => { ...@@ -62,6 +63,18 @@ describe('RichHistoryCard', () => {
.text() .text()
).toEqual('query3'); ).toEqual('query3');
}); });
it('should render data source icon', () => {
const wrapper = setup();
expect(wrapper.find({ 'aria-label': 'Data source icon' })).toHaveLength(1);
});
it('should render data source name', () => {
const wrapper = setup();
expect(wrapper.find({ 'aria-label': 'Data source name' }).text()).toEqual('Test datasource');
});
it('should render "Not linked to existing data source" if removed data source', () => {
const wrapper = setup({ isRemoved: true });
expect(wrapper.find({ 'aria-label': 'Data source name' }).text()).toEqual('Not linked to existing data source');
});
describe('commenting', () => { describe('commenting', () => {
it('should render comment, if comment present', () => { it('should render comment, if comment present', () => {
......
...@@ -8,7 +8,6 @@ import { RichHistoryQuery, ExploreId } from 'app/types/explore'; ...@@ -8,7 +8,6 @@ import { RichHistoryQuery, ExploreId } from 'app/types/explore';
// Utils // Utils
import { stylesFactory, useTheme } from '@grafana/ui'; import { stylesFactory, useTheme } from '@grafana/ui';
import { GrafanaTheme, SelectableValue } from '@grafana/data'; import { GrafanaTheme, SelectableValue } from '@grafana/data';
import { getExploreDatasources } from '../state/selectors';
import { SortOrder } from 'app/core/utils/explore'; import { SortOrder } from 'app/core/utils/explore';
import { import {
...@@ -16,6 +15,7 @@ import { ...@@ -16,6 +15,7 @@ import {
mapNumbertoTimeInSlider, mapNumbertoTimeInSlider,
createRetentionPeriodBoundary, createRetentionPeriodBoundary,
mapQueriesToHeadings, mapQueriesToHeadings,
createDatasourcesList,
} from 'app/core/utils/richHistory'; } from 'app/core/utils/richHistory';
// Components // Components
...@@ -136,14 +136,8 @@ export function RichHistoryQueriesTab(props: Props) { ...@@ -136,14 +136,8 @@ export function RichHistoryQueriesTab(props: Props) {
const theme = useTheme(); const theme = useTheme();
const styles = getStyles(theme, height); const styles = getStyles(theme, height);
const listOfDsNamesWithQueries = uniqBy(queries, 'datasourceName').map(d => d.datasourceName); const datasourcesRetrievedFromQueryHistory = uniqBy(queries, 'datasourceName').map(d => d.datasourceName);
const listOfDatasources = createDatasourcesList(datasourcesRetrievedFromQueryHistory);
/* Display only explore datasoources, that have saved queries */
const datasources = getExploreDatasources()
?.filter(ds => listOfDsNamesWithQueries.includes(ds.name))
.map(d => {
return { value: d.value!, label: d.value!, imgUrl: d.meta.info.logos.small };
});
const listOfDatasourceFilters = datasourceFilters?.map(d => d.value); const listOfDatasourceFilters = datasourceFilters?.map(d => d.value);
const filteredQueriesByDatasource = datasourceFilters const filteredQueriesByDatasource = datasourceFilters
...@@ -193,7 +187,7 @@ export function RichHistoryQueriesTab(props: Props) { ...@@ -193,7 +187,7 @@ export function RichHistoryQueriesTab(props: Props) {
<div aria-label="Filter datasources" className={styles.multiselect}> <div aria-label="Filter datasources" className={styles.multiselect}>
<Select <Select
isMulti={true} isMulti={true}
options={datasources} options={listOfDatasources}
value={datasourceFilters} value={datasourceFilters}
placeholder="Filter queries for specific data sources(s)" placeholder="Filter queries for specific data sources(s)"
onChange={onSelectDatasourceFilters} onChange={onSelectDatasourceFilters}
...@@ -215,9 +209,18 @@ export function RichHistoryQueriesTab(props: Props) { ...@@ -215,9 +209,18 @@ export function RichHistoryQueriesTab(props: Props) {
<div className={styles.heading}> <div className={styles.heading}>
{heading} <span className={styles.queries}>{mappedQueriesToHeadings[heading].length} queries</span> {heading} <span className={styles.queries}>{mappedQueriesToHeadings[heading].length} queries</span>
</div> </div>
{mappedQueriesToHeadings[heading].map((q: RichHistoryQuery) => ( {mappedQueriesToHeadings[heading].map((q: RichHistoryQuery) => {
<RichHistoryCard query={q} key={q.ts} exploreId={exploreId} /> const idx = listOfDatasources.findIndex(d => d.label === q.datasourceName);
))} return (
<RichHistoryCard
query={q}
key={q.ts}
exploreId={exploreId}
dsImg={listOfDatasources[idx].imgUrl}
isRemoved={listOfDatasources[idx].isRemoved}
/>
);
})}
</div> </div>
); );
})} })}
......
...@@ -8,10 +8,9 @@ import { RichHistoryQuery, ExploreId } from 'app/types/explore'; ...@@ -8,10 +8,9 @@ import { RichHistoryQuery, ExploreId } from 'app/types/explore';
// Utils // Utils
import { stylesFactory, useTheme } from '@grafana/ui'; import { stylesFactory, useTheme } from '@grafana/ui';
import { GrafanaTheme, SelectableValue } from '@grafana/data'; import { GrafanaTheme, SelectableValue } from '@grafana/data';
import { getExploreDatasources } from '../state/selectors';
import { SortOrder } from '../../../core/utils/explore'; import { SortOrder } from '../../../core/utils/explore';
import { sortQueries } from '../../../core/utils/richHistory'; import { sortQueries, createDatasourcesList } from '../../../core/utils/richHistory';
// Components // Components
import RichHistoryCard from './RichHistoryCard'; import RichHistoryCard from './RichHistoryCard';
...@@ -33,17 +32,6 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => { ...@@ -33,17 +32,6 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => {
return { return {
container: css` container: css`
display: flex; display: flex;
.label-slider {
font-size: ${theme.typography.size.sm};
&:last-of-type {
margin-top: ${theme.spacing.lg};
}
&:first-of-type {
margin-top: ${theme.spacing.sm};
font-weight: ${theme.typography.weight.semibold};
margin-bottom: ${theme.spacing.xs};
}
}
`, `,
containerContent: css` containerContent: css`
width: 100%; width: 100%;
...@@ -63,19 +51,18 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => { ...@@ -63,19 +51,18 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => {
sort: css` sort: css`
width: 170px; width: 170px;
`, `,
sessionName: css` feedback: css`
display: flex; height: 60px;
align-items: flex-start;
justify-content: flex-start;
margin-top: ${theme.spacing.lg}; margin-top: ${theme.spacing.lg};
h4 { display: flex;
margin: 0 10px 0 0; justify-content: center;
font-weight: ${theme.typography.weight.light};
font-size: ${theme.typography.size.sm};
a {
font-weight: ${theme.typography.weight.semibold};
margin-left: ${theme.spacing.xxs};
} }
`, `,
heading: css`
font-size: ${theme.typography.heading.h4};
margin: ${theme.spacing.md} ${theme.spacing.xxs} ${theme.spacing.sm} ${theme.spacing.xxs};
`,
}; };
}); });
...@@ -92,18 +79,17 @@ export function RichHistoryStarredTab(props: Props) { ...@@ -92,18 +79,17 @@ export function RichHistoryStarredTab(props: Props) {
const theme = useTheme(); const theme = useTheme();
const styles = getStyles(theme); const styles = getStyles(theme);
const listOfDsNamesWithQueries = uniqBy(queries, 'datasourceName').map(d => d.datasourceName);
const exploreDatasources = getExploreDatasources() const datasourcesRetrievedFromQueryHistory = uniqBy(queries, 'datasourceName').map(d => d.datasourceName);
?.filter(ds => listOfDsNamesWithQueries.includes(ds.name)) const listOfDatasources = createDatasourcesList(datasourcesRetrievedFromQueryHistory);
.map(d => {
return { value: d.value!, label: d.value!, imgUrl: d.meta.info.logos.small };
});
const listOfDatasourceFilters = datasourceFilters?.map(d => d.value); const listOfDatasourceFilters = datasourceFilters?.map(d => d.value);
const starredQueries = queries.filter(q => q.starred === true); const starredQueries = queries.filter(q => q.starred === true);
const starredQueriesFilteredByDatasource = datasourceFilters const starredQueriesFilteredByDatasource = datasourceFilters
? starredQueries?.filter(q => listOfDatasourceFilters?.includes(q.datasourceName)) ? starredQueries?.filter(q => listOfDatasourceFilters?.includes(q.datasourceName))
: starredQueries; : starredQueries;
const sortedStarredQueries = sortQueries(starredQueriesFilteredByDatasource, sortOrder); const sortedStarredQueries = sortQueries(starredQueriesFilteredByDatasource, sortOrder);
return ( return (
...@@ -114,7 +100,7 @@ export function RichHistoryStarredTab(props: Props) { ...@@ -114,7 +100,7 @@ export function RichHistoryStarredTab(props: Props) {
<div aria-label="Filter datasources" className={styles.multiselect}> <div aria-label="Filter datasources" className={styles.multiselect}>
<Select <Select
isMulti={true} isMulti={true}
options={exploreDatasources} options={listOfDatasources}
value={datasourceFilters} value={datasourceFilters}
placeholder="Filter queries for specific data sources(s)" placeholder="Filter queries for specific data sources(s)"
onChange={onSelectDatasourceFilters} onChange={onSelectDatasourceFilters}
...@@ -131,8 +117,21 @@ export function RichHistoryStarredTab(props: Props) { ...@@ -131,8 +117,21 @@ export function RichHistoryStarredTab(props: Props) {
</div> </div>
</div> </div>
{sortedStarredQueries.map(q => { {sortedStarredQueries.map(q => {
return <RichHistoryCard query={q} key={q.ts} exploreId={exploreId} />; const idx = listOfDatasources.findIndex(d => d.label === q.datasourceName);
return (
<RichHistoryCard
query={q}
key={q.ts}
exploreId={exploreId}
dsImg={listOfDatasources[idx].imgUrl}
isRemoved={listOfDatasources[idx].isRemoved}
/>
);
})} })}
<div className={styles.feedback}>
Query history is a beta feature. The history is local to your browser and is not shared with others.
<a href="https://github.com/grafana/grafana/issues/new/choose">Feedback?</a>
</div>
</div> </div>
</div> </div>
); );
......
...@@ -43,6 +43,7 @@ import { ...@@ -43,6 +43,7 @@ import {
deleteAllFromRichHistory, deleteAllFromRichHistory,
updateStarredInRichHistory, updateStarredInRichHistory,
updateCommentInRichHistory, updateCommentInRichHistory,
deleteQueryInRichHistory,
getQueryDisplayText, getQueryDisplayText,
getRichHistory, getRichHistory,
} from 'app/core/utils/richHistory'; } from 'app/core/utils/richHistory';
...@@ -525,6 +526,9 @@ export const updateRichHistory = (ts: number, property: string, updatedProperty? ...@@ -525,6 +526,9 @@ export const updateRichHistory = (ts: number, property: string, updatedProperty?
if (property === 'comment') { if (property === 'comment') {
nextRichHistory = updateCommentInRichHistory(getState().explore.richHistory, ts, updatedProperty); nextRichHistory = updateCommentInRichHistory(getState().explore.richHistory, ts, updatedProperty);
} }
if (property === 'delete') {
nextRichHistory = deleteQueryInRichHistory(getState().explore.richHistory, ts);
}
dispatch(richHistoryUpdatedAction({ richHistory: nextRichHistory })); dispatch(richHistoryUpdatedAction({ richHistory: nextRichHistory }));
}; };
}; };
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment