Migrate to UID: Stop using search result ID (#54099)

* DashList: Stop using IDs

* DashLinks: Stop using IDs

* BackendSrv: Do not use ID for search endpoint

* DashboardDataDTO: Remove ID

* Remove unused properties from DashboardSearchItem
This commit is contained in:
Ivan Ortega Alba 2022-08-26 09:42:46 +02:00 committed by GitHub
parent e27769da9c
commit c332bf885c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 90 additions and 65 deletions

View File

@ -5,6 +5,7 @@ import React, { useCallback, useMemo, useState } from 'react';
import { GrafanaTheme2, SelectableValue } from '@grafana/data';
import { AsyncMultiSelect, Icon, Button, useStyles2 } from '@grafana/ui';
import { getBackendSrv } from 'app/core/services/backend_srv';
import { DashboardSearchHit } from 'app/features/search/types';
import { FolderInfo, PermissionLevelString } from 'app/types';
export interface FolderFilterProps {
@ -75,7 +76,9 @@ async function getFoldersAsOptions(searchString: string, setLoading: (loading: b
permission: PermissionLevelString.View,
};
const searchHits = await getBackendSrv().search(params);
// FIXME: stop using id from search and use UID instead
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
const searchHits = (await getBackendSrv().search(params)) as DashboardSearchHit[];
const options = searchHits.map((d) => ({ label: d.title, value: { id: d.id, title: d.title } }));
if (!searchString || 'general'.includes(searchString.toLowerCase())) {
options.unshift({ label: 'General', value: { id: 0, title: 'General' } });

View File

@ -4,6 +4,7 @@ import React, { FC } from 'react';
import { SelectableValue } from '@grafana/data';
import { AsyncSelect } from '@grafana/ui';
import { backendSrv } from 'app/core/services/backend_srv';
import { DashboardSearchHit } from 'app/features/search/types';
/**
* @deprecated prefer using dashboard uid rather than id
@ -70,10 +71,12 @@ async function getDashboards(
label: string,
excludedDashboards?: string[]
): Promise<Array<SelectableValue<DashboardPickerItem>>> {
const result = await backendSrv.search({ type: 'dash-db', query, limit: 100 });
// FIXME: stop using id from search and use UID instead
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
const result = (await backendSrv.search({ type: 'dash-db', query, limit: 100 })) as DashboardSearchHit[];
const dashboards = result.map(({ id, uid = '', title, folderTitle }) => {
const value: DashboardPickerItem = {
id,
id: id!,
uid,
[label]: `${folderTitle ?? 'General'}/${title}`,
};

View File

@ -4,7 +4,7 @@ import React, { useCallback, useEffect, useState } from 'react';
import { SelectableValue } from '@grafana/data';
import { AsyncSelectProps, AsyncSelect } from '@grafana/ui';
import { backendSrv } from 'app/core/services/backend_srv';
import { DashboardSearchHit } from 'app/features/search/types';
import { DashboardSearchItem } from 'app/features/search/types';
import { DashboardDTO } from 'app/types';
interface Props extends Omit<AsyncSelectProps<DashboardPickerDTO>, 'value' | 'onChange' | 'loadOptions' | ''> {
@ -18,8 +18,8 @@ export type DashboardPickerDTO = Pick<DashboardDTO['dashboard'], 'uid' | 'title'
const formatLabel = (folderTitle = 'General', dashboardTitle: string) => `${folderTitle}/${dashboardTitle}`;
const getDashboards = debounce((query = ''): Promise<Array<SelectableValue<DashboardPickerDTO>>> => {
return backendSrv.search({ type: 'dash-db', query, limit: 100 }).then((result: DashboardSearchHit[]) => {
return result.map((item: DashboardSearchHit) => ({
return backendSrv.search({ type: 'dash-db', query, limit: 100 }).then((result: DashboardSearchItem[]) => {
return result.map((item: DashboardSearchItem) => ({
value: {
// dashboards uid here is always defined as this endpoint does not return the default home dashboard
uid: item.uid!,

View File

@ -1,13 +1,13 @@
import { silenceConsoleOutput } from '../../../../../test/core/utils/silenceConsoleOutput';
import * as api from '../../../../features/manage-dashboards/state/actions';
import { DashboardSearchHit } from '../../../../features/search/types';
import { DashboardSearchItem } from '../../../../features/search/types';
import { PermissionLevelString } from '../../../../types';
import { ALL_FOLDER, GENERAL_FOLDER } from './ReadonlyFolderPicker';
import { getFolderAsOption, getFoldersAsOptions } from './api';
function getTestContext(
searchHits: DashboardSearchHit[] = [],
searchHits: DashboardSearchItem[] = [],
folderById: { id: number; title: string } = { id: 1, title: 'Folder 1' }
) {
jest.clearAllMocks();

View File

@ -32,8 +32,10 @@ export interface Props {
disabled?: boolean;
}
type DefaultDashboardSearchItem = Omit<DashboardSearchItem, 'uid'> & { uid?: string };
export type State = UserPreferencesDTO & {
dashboards: DashboardSearchItem[];
dashboards: DashboardSearchItem[] | DefaultDashboardSearchItem[];
};
const themes: SelectableValue[] = [
@ -75,14 +77,13 @@ const languages: Array<SelectableValue<string>> = [
const i18nFlag = Boolean(config.featureToggles.internationalization);
const DEFAULT_DASHBOARD_HOME: DashboardSearchItem = {
const DEFAULT_DASHBOARD_HOME: DefaultDashboardSearchItem = {
title: 'Default',
tags: [],
type: '' as DashboardSearchItemType,
uid: undefined,
uri: '',
url: '',
folderId: 0,
folderTitle: '',
folderUid: '',
folderUrl: '',

View File

@ -18,7 +18,7 @@ import { BackendSrv as BackendService, BackendSrvRequest, config, FetchError, Fe
import appEvents from 'app/core/app_events';
import { getConfig } from 'app/core/config';
import { loadUrlToken } from 'app/core/utils/urlToken';
import { DashboardSearchHit } from 'app/features/search/types';
import { DashboardSearchItem } from 'app/features/search/types';
import { getGrafanaStorage } from 'app/features/storage/storage';
import { TokenRevokedModal } from 'app/features/users/TokenRevokedModal';
import { DashboardDTO, FolderDTO } from 'app/types';
@ -439,7 +439,7 @@ export class BackendSrv implements BackendService {
}
/** @deprecated */
search(query: any): Promise<DashboardSearchHit[]> {
search(query: any): Promise<DashboardSearchItem[]> {
return this.get('/api/search', query);
}

View File

@ -113,7 +113,8 @@ export class SearchSrv {
// create folder index
for (const hit of results) {
if (hit.type === 'dash-folder') {
sections[hit.id] = {
// FIXME: Use hit.uid instead
sections[hit.id!] = {
id: hit.id,
uid: hit.uid,
title: hit.title,

View File

@ -1,7 +1,7 @@
import { contextSrv } from 'app/core/services/context_srv';
import impressionSrv from 'app/core/services/impression_srv';
import { SearchSrv } from 'app/core/services/search_srv';
import { DashboardSearchHit } from 'app/features/search/types';
import { DashboardSearchItem } from 'app/features/search/types';
import { backendSrv } from '../services/backend_srv';
@ -40,7 +40,7 @@ describe('SearchSrv', () => {
return Promise.resolve([
{ uid: 'DSNdW0gVk', title: 'second but first' },
{ uid: 'srx16xR4z', title: 'first but second' },
] as DashboardSearchHit[]);
] as DashboardSearchItem[]);
}
return Promise.resolve([]);
});
@ -70,7 +70,7 @@ describe('SearchSrv', () => {
return Promise.resolve([
{ uid: 'DSNdW0gVk', title: 'two' },
{ uid: 'srx16xR4z', title: 'one' },
] as DashboardSearchHit[]);
] as DashboardSearchItem[]);
}
return Promise.resolve([]);
});
@ -98,7 +98,7 @@ describe('SearchSrv', () => {
beforeEach(() => {
searchMock.mockImplementation((options) => {
if (options.starred) {
return Promise.resolve([{ id: 1, title: 'starred' }] as DashboardSearchHit[]);
return Promise.resolve([{ uid: '1', title: 'starred' }] as DashboardSearchItem[]);
}
return Promise.resolve([]);
});
@ -123,9 +123,9 @@ describe('SearchSrv', () => {
return Promise.resolve([
{ uid: 'srx16xR4z', title: 'starred and recent', isStarred: true },
{ uid: 'DSNdW0gVk', title: 'recent' },
] as DashboardSearchHit[]);
] as DashboardSearchItem[]);
}
return Promise.resolve([{ uid: 'srx16xR4z', title: 'starred and recent' }] as DashboardSearchHit[]);
return Promise.resolve([{ uid: 'srx16xR4z', title: 'starred and recent' }] as DashboardSearchItem[]);
});
impressionSrv.getDashboardOpened = jest.fn().mockResolvedValue(['srx16xR4z', 'DSNdW0gVk']);

View File

@ -37,7 +37,7 @@ export const DashboardLinks: FC<Props> = ({ dashboard, links }) => {
const key = `${link.title}-$${index}`;
if (link.type === 'dashboards') {
return <DashboardLinksDashboard key={key} link={link} linkInfo={linkInfo} dashboardId={dashboard.id} />;
return <DashboardLinksDashboard key={key} link={link} linkInfo={linkInfo} dashboardUID={dashboard.uid} />;
}
const linkElement = (

View File

@ -1,4 +1,4 @@
import { DashboardSearchHit, DashboardSearchItemType } from '../../../search/types';
import { DashboardSearchItem, DashboardSearchItemType } from '../../../search/types';
import { DashboardLink } from '../../state/DashboardModel';
import { resolveLinks, searchForTags } from './DashboardLinksDashboard';
@ -39,7 +39,7 @@ describe('searchForTags', () => {
});
describe('resolveLinks', () => {
const setupTestContext = (dashboardId: number, searchHitId: number) => {
const setupTestContext = (dashboardUID: string, searchHitId: string) => {
const link: DashboardLink = {
targetBlank: false,
keepTime: false,
@ -52,9 +52,9 @@ describe('resolveLinks', () => {
type: 'dashboards',
url: '/d/6ieouugGk/DashLinks',
};
const searchHits: DashboardSearchHit[] = [
const searchHits: DashboardSearchItem[] = [
{
id: searchHitId,
uid: searchHitId,
title: 'DashLinks',
url: '/d/6ieouugGk/DashLinks',
isStarred: false,
@ -70,14 +70,18 @@ describe('resolveLinks', () => {
const sanitize = jest.fn((args) => args);
const sanitizeUrl = jest.fn((args) => args);
return { dashboardId, link, searchHits, linkSrv, sanitize, sanitizeUrl };
return { dashboardUID, link, searchHits, linkSrv, sanitize, sanitizeUrl };
};
describe('when called', () => {
it('should filter out the calling dashboardId', () => {
const { dashboardId, link, searchHits, linkSrv, sanitize, sanitizeUrl } = setupTestContext(1, 1);
it('should filter out the calling dashboardUID', () => {
const { dashboardUID, link, searchHits, linkSrv, sanitize, sanitizeUrl } = setupTestContext('1', '1');
const results = resolveLinks(dashboardId, link, searchHits, { getLinkSrv: () => linkSrv, sanitize, sanitizeUrl });
const results = resolveLinks(dashboardUID, link, searchHits, {
getLinkSrv: () => linkSrv,
sanitize,
sanitizeUrl,
});
expect(results.length).toEqual(0);
expect(linkSrv.getLinkUrl).toHaveBeenCalledTimes(0);
@ -86,9 +90,13 @@ describe('resolveLinks', () => {
});
it('should resolve link url', () => {
const { dashboardId, link, searchHits, linkSrv, sanitize, sanitizeUrl } = setupTestContext(1, 2);
const { dashboardUID, link, searchHits, linkSrv, sanitize, sanitizeUrl } = setupTestContext('1', '2');
const results = resolveLinks(dashboardId, link, searchHits, { getLinkSrv: () => linkSrv, sanitize, sanitizeUrl });
const results = resolveLinks(dashboardUID, link, searchHits, {
getLinkSrv: () => linkSrv,
sanitize,
sanitizeUrl,
});
expect(results.length).toEqual(1);
expect(linkSrv.getLinkUrl).toHaveBeenCalledTimes(1);
@ -96,9 +104,13 @@ describe('resolveLinks', () => {
});
it('should sanitize title', () => {
const { dashboardId, link, searchHits, linkSrv, sanitize, sanitizeUrl } = setupTestContext(1, 2);
const { dashboardUID, link, searchHits, linkSrv, sanitize, sanitizeUrl } = setupTestContext('1', '2');
const results = resolveLinks(dashboardId, link, searchHits, { getLinkSrv: () => linkSrv, sanitize, sanitizeUrl });
const results = resolveLinks(dashboardUID, link, searchHits, {
getLinkSrv: () => linkSrv,
sanitize,
sanitizeUrl,
});
expect(results.length).toEqual(1);
expect(sanitize).toHaveBeenCalledTimes(1);
@ -106,9 +118,13 @@ describe('resolveLinks', () => {
});
it('should sanitize url', () => {
const { dashboardId, link, searchHits, linkSrv, sanitize, sanitizeUrl } = setupTestContext(1, 2);
const { dashboardUID, link, searchHits, linkSrv, sanitize, sanitizeUrl } = setupTestContext('1', '2');
const results = resolveLinks(dashboardId, link, searchHits, { getLinkSrv: () => linkSrv, sanitize, sanitizeUrl });
const results = resolveLinks(dashboardUID, link, searchHits, {
getLinkSrv: () => linkSrv,
sanitize,
sanitizeUrl,
});
expect(results.length).toEqual(1);
expect(sanitizeUrl).toHaveBeenCalledTimes(1);

View File

@ -7,7 +7,7 @@ import { sanitize, sanitizeUrl } from '@grafana/data/src/text/sanitize';
import { selectors } from '@grafana/e2e-selectors';
import { Icon, ToolbarButton, Tooltip, useStyles2 } from '@grafana/ui';
import { getBackendSrv } from 'app/core/services/backend_srv';
import { DashboardSearchHit } from 'app/features/search/types';
import { DashboardSearchItem } from 'app/features/search/types';
import { getLinkSrv } from '../../../panel/panellinks/link_srv';
import { DashboardLink } from '../../state/DashboardModel';
@ -15,7 +15,7 @@ import { DashboardLink } from '../../state/DashboardModel';
interface Props {
link: DashboardLink;
linkInfo: { title: string; href: string };
dashboardId: number;
dashboardUID: string;
}
export const DashboardLinksDashboard = (props: Props) => {
@ -55,7 +55,7 @@ export const DashboardLinksDashboard = (props: Props) => {
{resolvedLinks.length > 0 &&
resolvedLinks.map((resolvedLink, index) => {
return (
<li role="none" key={`dashlinks-dropdown-item-${resolvedLink.id}-${index}`}>
<li role="none" key={`dashlinks-dropdown-item-${resolvedLink.uid}-${index}`}>
<a
role="menuitem"
href={resolvedLink.url}
@ -82,7 +82,7 @@ export const DashboardLinksDashboard = (props: Props) => {
return (
<LinkElement
link={link}
key={`dashlinks-list-item-${resolvedLink.id}-${index}`}
key={`dashlinks-list-item-${resolvedLink.uid}-${index}`}
data-testid={selectors.components.DashboardLinks.container}
>
<a
@ -120,17 +120,17 @@ const LinkElement: React.FC<LinkElementProps> = (props) => {
);
};
const useResolvedLinks = ({ link, dashboardId }: Props, opened: number): ResolvedLinkDTO[] => {
const useResolvedLinks = ({ link, dashboardUID }: Props, opened: number): ResolvedLinkDTO[] => {
const { tags } = link;
const result = useAsync(() => searchForTags(tags), [tags, opened]);
if (!result.value) {
return [];
}
return resolveLinks(dashboardId, link, result.value);
return resolveLinks(dashboardUID, link, result.value);
};
interface ResolvedLinkDTO {
id: number;
uid: string;
url: string;
title: string;
}
@ -138,17 +138,17 @@ interface ResolvedLinkDTO {
export async function searchForTags(
tags: string[],
dependencies: { getBackendSrv: typeof getBackendSrv } = { getBackendSrv }
): Promise<DashboardSearchHit[]> {
): Promise<DashboardSearchItem[]> {
const limit = 100;
const searchHits: DashboardSearchHit[] = await dependencies.getBackendSrv().search({ tag: tags, limit });
const searchHits: DashboardSearchItem[] = await dependencies.getBackendSrv().search({ tag: tags, limit });
return searchHits;
}
export function resolveLinks(
dashboardId: number,
dashboardUID: string,
link: DashboardLink,
searchHits: DashboardSearchHit[],
searchHits: DashboardSearchItem[],
dependencies: { getLinkSrv: typeof getLinkSrv; sanitize: typeof sanitize; sanitizeUrl: typeof sanitizeUrl } = {
getLinkSrv,
sanitize,
@ -156,14 +156,14 @@ export function resolveLinks(
}
): ResolvedLinkDTO[] {
return searchHits
.filter((searchHit) => searchHit.id !== dashboardId)
.filter((searchHit) => searchHit.uid !== dashboardUID)
.map((searchHit) => {
const id = searchHit.id;
const uid = searchHit.uid;
const title = dependencies.sanitize(searchHit.title);
const resolvedLink = dependencies.getLinkSrv().getLinkUrl({ ...link, url: searchHit.url });
const url = dependencies.sanitizeUrl(resolvedLink);
return { id, title, url };
return { uid, title, url };
});
}

View File

@ -199,7 +199,6 @@ describe('AddToDashboardButton', () => {
});
jest.spyOn(backendSrv, 'search').mockResolvedValue([
{
id: 1,
uid: 'someUid',
isStarred: false,
items: [],
@ -242,7 +241,6 @@ describe('AddToDashboardButton', () => {
});
jest.spyOn(backendSrv, 'search').mockResolvedValue([
{
id: 1,
uid: 'someUid',
isStarred: false,
items: [],
@ -359,7 +357,6 @@ describe('AddToDashboardButton', () => {
jest.spyOn(backendSrv, 'getDashboardByUid').mockRejectedValue('SOME ERROR');
jest.spyOn(backendSrv, 'search').mockResolvedValue([
{
id: 1,
uid: 'someUid',
isStarred: false,
items: [],

View File

@ -12,7 +12,7 @@ export enum DashboardSearchItemType {
* @deprecated
*/
export interface DashboardSection {
id: number;
id?: number;
uid?: string;
title: string;
expanded?: boolean;
@ -37,7 +37,7 @@ export interface DashboardSectionItem {
folderTitle?: string;
folderUid?: string;
folderUrl?: string;
id: number;
id?: number;
isStarred: boolean;
selected?: boolean;
tags: string[];
@ -56,7 +56,13 @@ export interface DashboardSectionItem {
*/
export interface DashboardSearchHit extends DashboardSectionItem, DashboardSection, WithAccessControlMetadata {}
export interface DashboardSearchItem extends Omit<DashboardSearchHit, 'id'> {}
export interface DashboardSearchItem
extends Omit<
DashboardSearchHit,
'id' | 'uid' | 'expanded' | 'selected' | 'checked' | 'folderId' | 'icon' | 'sortMeta' | 'sortMetaName'
> {
uid: string;
}
export interface SearchAction extends Action {
payload?: any;

View File

@ -12,12 +12,12 @@ import { getBackendSrv } from 'app/core/services/backend_srv';
import impressionSrv from 'app/core/services/impression_srv';
import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv';
import { SearchCard } from 'app/features/search/components/SearchCard';
import { DashboardSearchHit } from 'app/features/search/types';
import { DashboardSearchItem } from 'app/features/search/types';
import { PanelLayout, PanelOptions } from './models.gen';
import { getStyles } from './styles';
type Dashboard = DashboardSearchHit & { isSearchResult?: boolean; isRecent?: boolean };
type Dashboard = DashboardSearchItem & { id?: number; isSearchResult?: boolean; isRecent?: boolean };
interface DashboardGroup {
show: boolean;
@ -26,13 +26,13 @@ interface DashboardGroup {
}
async function fetchDashboards(options: PanelOptions, replaceVars: InterpolateFunction) {
let starredDashboards: Promise<Dashboard[]> = Promise.resolve([]);
let starredDashboards: Promise<DashboardSearchItem[]> = Promise.resolve([]);
if (options.showStarred) {
const params = { limit: options.maxItems, starred: 'true' };
starredDashboards = getBackendSrv().search(params);
}
let recentDashboards: Promise<Dashboard[]> = Promise.resolve([]);
let recentDashboards: Promise<DashboardSearchItem[]> = Promise.resolve([]);
let dashUIDs: string[] = [];
if (options.showRecentlyViewed) {
let uids = await impressionSrv.getDashboardOpened();
@ -40,7 +40,7 @@ async function fetchDashboards(options: PanelOptions, replaceVars: InterpolateFu
recentDashboards = getBackendSrv().search({ dashboardUIDs: dashUIDs, limit: options.maxItems });
}
let searchedDashboards: Promise<Dashboard[]> = Promise.resolve([]);
let searchedDashboards: Promise<DashboardSearchItem[]> = Promise.resolve([]);
if (options.showSearch) {
const params = {
limit: options.maxItems,
@ -103,7 +103,8 @@ export function DashList(props: PanelProps<PanelOptions>) {
e.preventDefault();
e.stopPropagation();
const isStarred = await getDashboardSrv().starDashboard(dash.id.toString(), dash.isStarred);
// FIXME: Do not use dash ID. Use UID to star a dashboard once the backend allows it
const isStarred = await getDashboardSrv().starDashboard(dash.id!.toString(), dash.isStarred);
const updatedDashboards = new Map(dashboards);
updatedDashboards.set(dash?.uid ?? '', { ...dash, isStarred });
setDashboards(updatedDashboards);
@ -144,7 +145,7 @@ export function DashList(props: PanelProps<PanelOptions>) {
const renderList = (dashboards: Dashboard[]) => (
<ul>
{dashboards.map((dash) => (
<li className={css.dashlistItem} key={`dash-${dash.id}`}>
<li className={css.dashlistItem} key={`dash-${dash.uid}`}>
<div className={css.dashlistLink}>
<div className={css.dashlistLinkBody}>
<a className={css.dashlistTitle} href={dash.url}>

View File

@ -65,9 +65,6 @@ export interface DashboardDataDTO {
list: VariableModel[];
};
panels?: any[];
/** @deprecated -- components should key on uid rather than id */
id?: number;
}
export enum DashboardRoutes {