Search: Migrated impressions to use dashboardUID (#53090)

* used dashboarduid in impressions

* handle scenario to convert all ids saved in storage to uids
This commit is contained in:
Leo 2022-08-08 11:13:19 +02:00 committed by GitHub
parent beb3cb9abe
commit 64967325b2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 186 additions and 115 deletions

View File

@ -4319,8 +4319,7 @@ exports[`better eslint`] = {
[0, 0, 0, "Unexpected any. Specify a different type.", "5"], [0, 0, 0, "Unexpected any. Specify a different type.", "5"],
[0, 0, 0, "Unexpected any. Specify a different type.", "6"], [0, 0, 0, "Unexpected any. Specify a different type.", "6"],
[0, 0, 0, "Unexpected any. Specify a different type.", "7"], [0, 0, 0, "Unexpected any. Specify a different type.", "7"],
[0, 0, 0, "Unexpected any. Specify a different type.", "8"], [0, 0, 0, "Unexpected any. Specify a different type.", "8"]
[0, 0, 0, "Unexpected any. Specify a different type.", "9"]
], ],
"public/app/features/dashboard/services/DashboardSrv.ts:5381": [ "public/app/features/dashboard/services/DashboardSrv.ts:5381": [
[0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

View File

@ -1,14 +1,15 @@
import { filter, isArray, isNumber } from 'lodash'; import { filter, isArray, isNumber, isString } from 'lodash';
import { getBackendSrv } from '@grafana/runtime';
import config from 'app/core/config'; import config from 'app/core/config';
import store from 'app/core/store'; import store from 'app/core/store';
export class ImpressionSrv { export class ImpressionSrv {
constructor() {} constructor() {}
addDashboardImpression(dashboardId: number) { addDashboardImpression(dashboardUID: string) {
const impressionsKey = this.impressionKey(); const impressionsKey = this.impressionKey();
let impressions = []; let impressions: string[] = [];
if (store.exists(impressionsKey)) { if (store.exists(impressionsKey)) {
impressions = JSON.parse(store.get(impressionsKey)); impressions = JSON.parse(store.get(impressionsKey));
if (!isArray(impressions)) { if (!isArray(impressions)) {
@ -17,10 +18,10 @@ export class ImpressionSrv {
} }
impressions = impressions.filter((imp) => { impressions = impressions.filter((imp) => {
return dashboardId !== imp; return dashboardUID !== imp;
}); });
impressions.unshift(dashboardId); impressions.unshift(dashboardUID);
if (impressions.length > 50) { if (impressions.length > 50) {
impressions.pop(); impressions.pop();
@ -28,17 +29,32 @@ export class ImpressionSrv {
store.set(impressionsKey, JSON.stringify(impressions)); store.set(impressionsKey, JSON.stringify(impressions));
} }
/** Returns an array of internal (numeric) dashboard IDs */ private async convertToUIDs() {
getDashboardOpened(): number[] { let impressions = this.getImpressions();
let impressions = store.get(this.impressionKey()) || '[]'; const ids = filter(impressions, (el) => isNumber(el));
if (!ids.length) {
return;
}
impressions = JSON.parse(impressions); const convertedUIDs = await getBackendSrv().get<string[]>(`/api/dashboards/ids/${ids.join(',')}`);
store.set(this.impressionKey(), JSON.stringify([...filter(impressions, (el) => isString(el)), ...convertedUIDs]));
}
impressions = filter(impressions, (el) => { private getImpressions() {
return isNumber(el); const impressions = store.get(this.impressionKey()) || '[]';
});
return impressions; return JSON.parse(impressions);
}
/** Returns an array of internal (string) dashboard UIDs */
async getDashboardOpened(): Promise<string[]> {
// TODO should be removed after UID migration
try {
await this.convertToUIDs();
} catch (_) {}
const result = filter(this.getImpressions(), (el) => isString(el));
return result;
} }
impressionKey() { impressionKey() {

View File

@ -31,15 +31,21 @@ export class SearchSrv {
} }
private queryForRecentDashboards(): Promise<DashboardSearchHit[]> { private queryForRecentDashboards(): Promise<DashboardSearchHit[]> {
const dashIds: number[] = take(impressionSrv.getDashboardOpened(), 30); return new Promise((resolve) => {
if (dashIds.length === 0) { impressionSrv.getDashboardOpened().then((uids) => {
return Promise.resolve([]); const dashUIDs: string[] = take(uids, 30);
} if (dashUIDs.length === 0) {
return resolve([]);
}
return backendSrv.search({ dashboardIds: dashIds }).then((result) => { backendSrv.search({ dashboardUIDs: dashUIDs }).then((result) => {
return dashIds return resolve(
.map((orderId) => result.find((result) => result.id === orderId)) dashUIDs
.filter((hit) => hit && !hit.isStarred) as DashboardSearchHit[]; .map((orderId) => result.find((result) => result.uid === orderId))
.filter((hit) => hit && !hit.isStarred) as DashboardSearchHit[]
);
});
});
}); });
} }

View File

@ -0,0 +1,45 @@
const mockBackendSrv = jest.fn();
import impressionSrv from '../services/impression_srv';
jest.mock('@grafana/runtime', () => {
const originalRuntime = jest.requireActual('@grafana/runtime');
return {
...originalRuntime,
getBackendSrv: mockBackendSrv,
config: {
...originalRuntime.config,
bootData: {
...originalRuntime.config.bootData,
user: {
...originalRuntime.config.bootData.user,
orgId: 'testOrgId',
},
},
},
};
});
describe('ImpressionSrv', () => {
beforeEach(() => {
window.localStorage.removeItem(impressionSrv.impressionKey());
});
describe('getDashboardOpened', () => {
it('should return list of dashboard uids', async () => {
window.localStorage.setItem(impressionSrv.impressionKey(), JSON.stringify(['five', 'four', 1, 2, 3]));
mockBackendSrv.mockImplementation(() => ({ get: jest.fn().mockResolvedValue(['one', 'two', 'three']) }));
const result1 = await impressionSrv.getDashboardOpened();
expect(result1).toEqual(['five', 'four', 'one', 'two', 'three']);
window.localStorage.setItem(impressionSrv.impressionKey(), JSON.stringify(['three', 'four']));
const result2 = await impressionSrv.getDashboardOpened();
expect(result2).toEqual(['three', 'four']);
window.localStorage.setItem(impressionSrv.impressionKey(), JSON.stringify([1, 2, 3]));
mockBackendSrv.mockImplementation(() => ({ get: jest.fn().mockResolvedValue(['one', 'two', 'three']) }));
const result3 = await impressionSrv.getDashboardOpened();
expect(result3).toEqual(['one', 'two', 'three']);
});
});
});

View File

@ -1,6 +1,7 @@
import { contextSrv } from 'app/core/services/context_srv'; import { contextSrv } from 'app/core/services/context_srv';
import impressionSrv from 'app/core/services/impression_srv'; import impressionSrv from 'app/core/services/impression_srv';
import { SearchSrv } from 'app/core/services/search_srv'; import { SearchSrv } from 'app/core/services/search_srv';
import { DashboardSearchHit } from 'app/features/search/types';
import { backendSrv } from '../services/backend_srv'; import { backendSrv } from '../services/backend_srv';
@ -26,7 +27,7 @@ describe('SearchSrv', () => {
searchSrv = new SearchSrv(); searchSrv = new SearchSrv();
contextSrv.isSignedIn = true; contextSrv.isSignedIn = true;
impressionSrv.getDashboardOpened = jest.fn().mockReturnValue([]); impressionSrv.getDashboardOpened = jest.fn().mockResolvedValue([]);
jest.clearAllMocks(); jest.clearAllMocks();
}); });
@ -34,19 +35,17 @@ describe('SearchSrv', () => {
let results: any; let results: any;
beforeEach(() => { beforeEach(() => {
searchMock.mockImplementation( searchMock.mockImplementation((options) => {
jest if (options.dashboardUIDs) {
.fn() return Promise.resolve([
.mockReturnValueOnce( { uid: 'DSNdW0gVk', title: 'second but first' },
Promise.resolve([ { uid: 'srx16xR4z', title: 'first but second' },
{ id: 2, title: 'second but first' }, ] as DashboardSearchHit[]);
{ id: 1, title: 'first but second' }, }
]) return Promise.resolve([]);
) });
.mockReturnValue(Promise.resolve([]))
);
impressionSrv.getDashboardOpened = jest.fn().mockReturnValue([1, 2]); impressionSrv.getDashboardOpened = jest.fn().mockResolvedValue(['srx16xR4z', 'DSNdW0gVk']);
return searchSrv.search({ query: '' }).then((res) => { return searchSrv.search({ query: '' }).then((res) => {
results = res; results = res;
@ -66,19 +65,19 @@ describe('SearchSrv', () => {
let results: any; let results: any;
beforeEach(() => { beforeEach(() => {
searchMock.mockImplementation( searchMock.mockImplementation((options) => {
jest if (options.dashboardUIDs) {
.fn() return Promise.resolve([
.mockReturnValueOnce( { uid: 'DSNdW0gVk', title: 'two' },
Promise.resolve([ { uid: 'srx16xR4z', title: 'one' },
{ id: 2, title: 'two' }, ] as DashboardSearchHit[]);
{ id: 1, title: 'one' }, }
]) return Promise.resolve([]);
) });
.mockReturnValue(Promise.resolve([]))
);
impressionSrv.getDashboardOpened = jest.fn().mockReturnValue([4, 5, 1, 2, 3]); impressionSrv.getDashboardOpened = jest
.fn()
.mockResolvedValue(['Xrx16x4z', 'CSxdW0gYA', 'srx16xR4z', 'DSNdW0gVk', 'xSxdW0gYA']);
return searchSrv.search({ query: '' }).then((res) => { return searchSrv.search({ query: '' }).then((res) => {
results = res; results = res;
@ -87,8 +86,8 @@ describe('SearchSrv', () => {
it('should return 2 dashboards', () => { it('should return 2 dashboards', () => {
expect(results[0].items.length).toBe(2); expect(results[0].items.length).toBe(2);
expect(results[0].items[0].id).toBe(1); expect(results[0].items[0].uid).toBe('srx16xR4z');
expect(results[0].items[1].id).toBe(2); expect(results[0].items[1].uid).toBe('DSNdW0gVk');
}); });
}); });
}); });
@ -97,7 +96,12 @@ describe('SearchSrv', () => {
let results: any; let results: any;
beforeEach(() => { beforeEach(() => {
searchMock.mockImplementation(jest.fn().mockReturnValue(Promise.resolve([{ id: 1, title: 'starred' }]))); searchMock.mockImplementation((options) => {
if (options.starred) {
return Promise.resolve([{ id: 1, title: 'starred' }] as DashboardSearchHit[]);
}
return Promise.resolve([]);
});
return searchSrv.search({ query: '' }).then((res) => { return searchSrv.search({ query: '' }).then((res) => {
results = res; results = res;
@ -114,19 +118,17 @@ describe('SearchSrv', () => {
let results: any; let results: any;
beforeEach(() => { beforeEach(() => {
searchMock.mockImplementation( searchMock.mockImplementation((options) => {
jest if (options.dashboardUIDs) {
.fn() return Promise.resolve([
.mockReturnValueOnce( { uid: 'srx16xR4z', title: 'starred and recent', isStarred: true },
Promise.resolve([ { uid: 'DSNdW0gVk', title: 'recent' },
{ id: 1, title: 'starred and recent', isStarred: true }, ] as DashboardSearchHit[]);
{ id: 2, title: 'recent' }, }
]) return Promise.resolve([{ uid: 'srx16xR4z', title: 'starred and recent' }] as DashboardSearchHit[]);
) });
.mockReturnValue(Promise.resolve([{ id: 1, title: 'starred and recent' }]))
);
impressionSrv.getDashboardOpened = jest.fn().mockReturnValue([1, 2]); impressionSrv.getDashboardOpened = jest.fn().mockResolvedValue(['srx16xR4z', 'DSNdW0gVk']);
return searchSrv.search({ query: '' }).then((res) => { return searchSrv.search({ query: '' }).then((res) => {
results = res; results = res;
}); });
@ -150,8 +152,8 @@ describe('SearchSrv', () => {
searchMock.mockImplementation( searchMock.mockImplementation(
jest jest
.fn() .fn()
.mockReturnValueOnce(Promise.resolve([])) .mockResolvedValueOnce(Promise.resolve([]))
.mockReturnValue( .mockResolvedValue(
Promise.resolve([ Promise.resolve([
{ {
title: 'folder1', title: 'folder1',
@ -198,24 +200,22 @@ describe('SearchSrv', () => {
beforeEach(() => { beforeEach(() => {
searchMock.mockImplementation( searchMock.mockImplementation(
jest.fn().mockReturnValue( jest.fn().mockResolvedValue([
Promise.resolve([ {
{ id: 2,
id: 2, title: 'dash with no folder',
title: 'dash with no folder', type: 'dash-db',
type: 'dash-db', },
}, {
{ id: 3,
id: 3, title: 'dash in folder1 1',
title: 'dash in folder1 1', type: 'dash-db',
type: 'dash-db', folderId: 1,
folderId: 1, folderUid: 'uid',
folderUid: 'uid', folderTitle: 'folder1',
folderTitle: 'folder1', folderUrl: '/dashboards/f/uid/folder1',
folderUrl: '/dashboards/f/uid/folder1', },
}, ])
])
)
); );
return searchSrv.search({ query: 'search' }).then((res) => { return searchSrv.search({ query: 'search' }).then((res) => {
@ -239,7 +239,7 @@ describe('SearchSrv', () => {
describe('with tags', () => { describe('with tags', () => {
beforeEach(() => { beforeEach(() => {
searchMock.mockImplementation(jest.fn().mockReturnValue(Promise.resolve([]))); searchMock.mockImplementation(jest.fn().mockResolvedValue(Promise.resolve([])));
return searchSrv.search({ tag: ['atag'] }).then(() => {}); return searchSrv.search({ tag: ['atag'] }).then(() => {});
}); });
@ -251,7 +251,7 @@ describe('SearchSrv', () => {
describe('with starred', () => { describe('with starred', () => {
beforeEach(() => { beforeEach(() => {
searchMock.mockImplementation(jest.fn().mockReturnValue(Promise.resolve([]))); searchMock.mockImplementation(jest.fn().mockResolvedValue(Promise.resolve([])));
return searchSrv.search({ starred: true }).then(() => {}); return searchSrv.search({ starred: true }).then(() => {});
}); });
@ -265,7 +265,7 @@ describe('SearchSrv', () => {
let getRecentDashboardsCalled = false; let getRecentDashboardsCalled = false;
beforeEach(() => { beforeEach(() => {
searchMock.mockImplementation(jest.fn().mockReturnValue(Promise.resolve([]))); searchMock.mockImplementation(jest.fn().mockResolvedValue(Promise.resolve([])));
searchSrv['getRecentDashboards'] = () => { searchSrv['getRecentDashboards'] = () => {
getRecentDashboardsCalled = true; getRecentDashboardsCalled = true;
@ -284,8 +284,8 @@ describe('SearchSrv', () => {
let getStarredCalled = false; let getStarredCalled = false;
beforeEach(() => { beforeEach(() => {
searchMock.mockImplementation(jest.fn().mockReturnValue(Promise.resolve([]))); searchMock.mockImplementation(jest.fn().mockResolvedValue(Promise.resolve([])));
impressionSrv.getDashboardOpened = jest.fn().mockReturnValue([]); impressionSrv.getDashboardOpened = jest.fn().mockResolvedValue([]);
searchSrv['getStarred'] = () => { searchSrv['getStarred'] = () => {
getStarredCalled = true; getStarredCalled = true;

View File

@ -9,7 +9,7 @@ import impressionSrv from 'app/core/services/impression_srv';
import kbn from 'app/core/utils/kbn'; import kbn from 'app/core/utils/kbn';
import { getDatasourceSrv } from 'app/features/plugins/datasource_srv'; import { getDatasourceSrv } from 'app/features/plugins/datasource_srv';
import { getGrafanaStorage } from 'app/features/storage/storage'; import { getGrafanaStorage } from 'app/features/storage/storage';
import { DashboardRoutes } from 'app/types'; import { DashboardDTO, DashboardRoutes } from 'app/types';
import { appEvents } from '../../../core/core'; import { appEvents } from '../../../core/core';
@ -69,9 +69,9 @@ export class DashboardLoaderSrv {
}); });
} }
promise.then((result: any) => { promise.then((result: DashboardDTO) => {
if (result.meta.dashboardNotFound !== true) { if (result.meta.dashboardNotFound !== true) {
impressionSrv.addDashboardImpression(result.dashboard.id); impressionSrv.addDashboardImpression(result.dashboard.uid);
} }
return result; return result;

View File

@ -99,7 +99,7 @@ describe('FolderView', () => {
}); });
it('does not show the recent items if no dashboards have been opened recently', async () => { it('does not show the recent items if no dashboards have been opened recently', async () => {
jest.spyOn(impressionSrv, 'getDashboardOpened').mockReturnValue([]); jest.spyOn(impressionSrv, 'getDashboardOpened').mockResolvedValue([]);
render( render(
<FolderView onTagSelected={mockOnTagSelected} selection={mockSelection} selectionToggle={mockSelectionToggle} /> <FolderView onTagSelected={mockOnTagSelected} selection={mockSelection} selectionToggle={mockSelectionToggle} />
); );
@ -108,7 +108,7 @@ describe('FolderView', () => {
}); });
it('shows the recent items if any dashboards have recently been opened', async () => { it('shows the recent items if any dashboards have recently been opened', async () => {
jest.spyOn(impressionSrv, 'getDashboardOpened').mockReturnValue([12345]); jest.spyOn(impressionSrv, 'getDashboardOpened').mockResolvedValue(['7MeksYbmk']);
render( render(
<FolderView onTagSelected={mockOnTagSelected} selection={mockSelection} selectionToggle={mockSelectionToggle} /> <FolderView onTagSelected={mockOnTagSelected} selection={mockSelection} selectionToggle={mockSelectionToggle} />
); );
@ -138,7 +138,7 @@ describe('FolderView', () => {
}); });
it('does not show the recent items even if recent dashboards have been opened', async () => { it('does not show the recent items even if recent dashboards have been opened', async () => {
jest.spyOn(impressionSrv, 'getDashboardOpened').mockReturnValue([12345]); jest.spyOn(impressionSrv, 'getDashboardOpened').mockResolvedValue(['7MeksYbmk']);
render( render(
<FolderView <FolderView
hidePseudoFolders hidePseudoFolders

View File

@ -39,12 +39,9 @@ export const FolderView = ({
} }
} }
const ids = impressionSrv.getDashboardOpened(); const itemsUIDs = await impressionSrv.getDashboardOpened();
if (ids.length) { if (itemsUIDs.length) {
const itemsUIDs = await getBackendSrv().get(`/api/dashboards/ids/${ids.slice(0, 30).join(',')}`); folders.push({ title: 'Recent', icon: 'clock-nine', kind: 'query-recent', uid: '__recent', itemsUIDs });
if (itemsUIDs.length) {
folders.push({ title: 'Recent', icon: 'clock-nine', kind: 'query-recent', uid: '__recent', itemsUIDs });
}
} }
} }
folders.push({ title: 'General', url: '/dashboards', kind: 'folder', uid: GENERAL_FOLDER_UID }); folders.push({ title: 'General', url: '/dashboards', kind: 'folder', uid: GENERAL_FOLDER_UID });

View File

@ -33,10 +33,11 @@ async function fetchDashboards(options: PanelOptions, replaceVars: InterpolateFu
} }
let recentDashboards: Promise<Dashboard[]> = Promise.resolve([]); let recentDashboards: Promise<Dashboard[]> = Promise.resolve([]);
let dashIds: number[] = []; let dashUIDs: string[] = [];
if (options.showRecentlyViewed) { if (options.showRecentlyViewed) {
dashIds = take<number>(impressionSrv.getDashboardOpened(), options.maxItems); let uids = await impressionSrv.getDashboardOpened();
recentDashboards = getBackendSrv().search({ dashboardIds: dashIds, limit: options.maxItems }); dashUIDs = take<string>(uids, options.maxItems);
recentDashboards = getBackendSrv().search({ dashboardUIDs: dashUIDs, limit: options.maxItems });
} }
let searchedDashboards: Promise<Dashboard[]> = Promise.resolve([]); let searchedDashboards: Promise<Dashboard[]> = Promise.resolve([]);
@ -55,27 +56,33 @@ async function fetchDashboards(options: PanelOptions, replaceVars: InterpolateFu
const [starred, searched, recent] = await Promise.all([starredDashboards, searchedDashboards, recentDashboards]); const [starred, searched, recent] = await Promise.all([starredDashboards, searchedDashboards, recentDashboards]);
// We deliberately deal with recent dashboards first so that the order of dash IDs is preserved // We deliberately deal with recent dashboards first so that the order of dash IDs is preserved
let dashMap = new Map<number, Dashboard>(); let dashMap = new Map<string, Dashboard>();
for (const dashId of dashIds) { for (const dashUID of dashUIDs) {
const dash = recent.find((d) => d.id === dashId); const dash = recent.find((d) => d.uid === dashUID);
if (dash) { if (dash) {
dashMap.set(dashId, { ...dash, isRecent: true }); dashMap.set(dashUID, { ...dash, isRecent: true });
} }
} }
searched.forEach((dash) => { searched.forEach((dash) => {
if (dashMap.has(dash.id)) { if (!dash.uid) {
dashMap.get(dash.id)!.isSearchResult = true; return;
}
if (dashMap.has(dash.uid)) {
dashMap.get(dash.uid)!.isSearchResult = true;
} else { } else {
dashMap.set(dash.id, { ...dash, isSearchResult: true }); dashMap.set(dash.uid, { ...dash, isSearchResult: true });
} }
}); });
starred.forEach((dash) => { starred.forEach((dash) => {
if (dashMap.has(dash.id)) { if (!dash.uid) {
dashMap.get(dash.id)!.isStarred = true; return;
}
if (dashMap.has(dash.uid)) {
dashMap.get(dash.uid)!.isStarred = true;
} else { } else {
dashMap.set(dash.id, { ...dash, isStarred: true }); dashMap.set(dash.uid, { ...dash, isStarred: true });
} }
}); });
@ -83,7 +90,7 @@ async function fetchDashboards(options: PanelOptions, replaceVars: InterpolateFu
} }
export function DashList(props: PanelProps<PanelOptions>) { export function DashList(props: PanelProps<PanelOptions>) {
const [dashboards, setDashboards] = useState(new Map<number, Dashboard>()); const [dashboards, setDashboards] = useState(new Map<string, Dashboard>());
const dispatch = useDispatch(); const dispatch = useDispatch();
useEffect(() => { useEffect(() => {
fetchDashboards(props.options, props.replaceVariables).then((dashes) => { fetchDashboards(props.options, props.replaceVariables).then((dashes) => {
@ -98,7 +105,7 @@ export function DashList(props: PanelProps<PanelOptions>) {
const isStarred = await getDashboardSrv().starDashboard(dash.id.toString(), dash.isStarred); const isStarred = await getDashboardSrv().starDashboard(dash.id.toString(), dash.isStarred);
const updatedDashboards = new Map(dashboards); const updatedDashboards = new Map(dashboards);
updatedDashboards.set(dash.id, { ...dash, isStarred }); updatedDashboards.set(dash?.uid ?? '', { ...dash, isStarred });
setDashboards(updatedDashboards); setDashboards(updatedDashboards);
dispatch(setStarred({ id: uid ?? '', title, url, isStarred })); dispatch(setStarred({ id: uid ?? '', title, url, isStarred }));
}; };

View File

@ -43,6 +43,7 @@ export interface DashboardMeta {
annotationsPermissions?: AnnotationsPermissions; annotationsPermissions?: AnnotationsPermissions;
publicDashboardAccessToken?: string; publicDashboardAccessToken?: string;
publicDashboardEnabled?: boolean; publicDashboardEnabled?: boolean;
dashboardNotFound?: boolean;
} }
export interface AnnotationActions { export interface AnnotationActions {