From ee65f895336ce0f65b382af0486d6be667fda814 Mon Sep 17 00:00:00 2001 From: Haris Rozajac <58232930+harisrozajac@users.noreply.github.com> Date: Tue, 8 Oct 2024 07:12:33 -0600 Subject: [PATCH] Query Library: Enhance user data (#94231) * Enhance user data * Remove irrrelevant logic outside of try block * Encode user uid for url --- .../QueryLibrary/QueryTemplatesList.tsx | 55 +++++++++++++++++-- .../QueryTemplatesTable/AddedByCell.tsx | 10 +++- .../QueryLibrary/QueryTemplatesTable/types.ts | 3 +- .../explore/QueryLibrary/utils/search.ts | 16 ++++-- .../features/explore/spec/helper/setup.tsx | 8 +-- .../query-library/api/mappers.test.ts | 18 ------ .../app/features/query-library/api/mappers.ts | 26 +-------- .../app/features/query-library/api/mocks.ts | 4 ++ .../api/testdata/identityDisplayList.ts | 26 +++++++++ .../api/testdata/testQueryList.ts | 4 +- .../app/features/query-library/api/types.ts | 25 +++++++++ public/app/features/query-library/api/user.ts | 18 ++++++ public/app/features/query-library/index.ts | 6 +- public/app/features/query-library/types.ts | 8 ++- public/locales/en-US/grafana.json | 4 +- public/locales/pseudo-LOCALE/grafana.json | 4 +- 16 files changed, 173 insertions(+), 62 deletions(-) delete mode 100644 public/app/features/query-library/api/mappers.test.ts create mode 100644 public/app/features/query-library/api/testdata/identityDisplayList.ts create mode 100644 public/app/features/query-library/api/user.ts diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesList.tsx b/public/app/features/explore/QueryLibrary/QueryTemplatesList.tsx index 00a8e518d87..ead35b647c6 100644 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesList.tsx +++ b/public/app/features/explore/QueryLibrary/QueryTemplatesList.tsx @@ -1,5 +1,5 @@ import { css } from '@emotion/css'; -import { uniqBy } from 'lodash'; +import { compact, uniq, uniqBy } from 'lodash'; import { useEffect, useMemo, useState } from 'react'; import { AppEvents, GrafanaTheme2, SelectableValue } from '@grafana/data'; @@ -8,6 +8,7 @@ import { EmptyState, FilterInput, InlineLabel, MultiSelect, Spinner, useStyles2, import { t, Trans } from 'app/core/internationalization'; import { createQueryText } from 'app/core/utils/richHistory'; import { useAllQueryTemplatesQuery } from 'app/features/query-library'; +import { getUserInfo } from 'app/features/query-library/api/user'; import { QueryTemplate } from 'app/features/query-library/types'; import { getDatasourceSrv } from '../../plugins/datasource_srv'; @@ -26,6 +27,8 @@ export function QueryTemplatesList(props: QueryTemplatesListProps) { const [datasourceFilters, setDatasourceFilters] = useState>>( props.activeDatasources?.map((ds) => ({ value: ds, label: ds })) || [] ); + const [userData, setUserData] = useState([]); + const [userFilters, setUserFilters] = useState>>([]); const [allQueryTemplateRows, setAllQueryTemplateRows] = useState([]); const [isRowsLoading, setIsRowsLoading] = useState(true); @@ -40,6 +43,26 @@ export function QueryTemplatesList(props: QueryTemplatesListProps) { return; } + let userDataList; + const userQtList = uniq(compact(data.map((qt) => qt.user?.uid))); + const usersParam = userQtList.map((userUid) => `key=${encodeURIComponent(userUid)}`).join('&'); + try { + userDataList = await getUserInfo(`?${usersParam}`); + } catch (error) { + getAppEvents().publish({ + type: AppEvents.alertError.name, + payload: [ + t('query-library.user-info-get-error', 'Error attempting to get user info from the library: {{error}}', { + error: JSON.stringify(error), + }), + ], + }); + setIsRowsLoading(false); + return; + } + + setUserData(userDataList.display.map((user) => user.displayName)); + const rowsPromises = data.map(async (queryTemplate: QueryTemplate, index: number) => { try { const datasourceRef = queryTemplate.targets[0]?.datasource; @@ -48,6 +71,9 @@ export function QueryTemplatesList(props: QueryTemplatesListProps) { const query = queryTemplate.targets[0]; const queryText = createQueryText(query, datasourceApi); const datasourceName = datasourceApi?.name || ''; + const extendedUserData = userDataList.display.find( + (user) => `${user?.identity.type}:${user?.identity.name}` === queryTemplate.user?.uid + ); return { index: index.toString(), @@ -59,7 +85,11 @@ export function QueryTemplatesList(props: QueryTemplatesListProps) { query, queryText, description: queryTemplate.title, - user: queryTemplate.user, + user: { + uid: queryTemplate.user?.uid || '', + displayName: extendedUserData?.displayName || '', + avatarUrl: extendedUserData?.avatarUrl || '', + }, }; } catch (error) { getAppEvents().publish({ @@ -97,9 +127,10 @@ export function QueryTemplatesList(props: QueryTemplatesListProps) { searchQueryLibrary( allQueryTemplateRows, searchQuery, - datasourceFilters.map((f) => f.value || '') + datasourceFilters.map((f) => f.value || ''), + userFilters.map((f) => f.value || '') ), - [allQueryTemplateRows, searchQuery, datasourceFilters] + [allQueryTemplateRows, searchQuery, datasourceFilters, userFilters] ); const datasourceNames = useMemo(() => { @@ -157,6 +188,22 @@ export function QueryTemplatesList(props: QueryTemplatesListProps) { placeholder={'Filter queries for data sources(s)'} aria-label={'Filter queries for data sources(s)'} /> + + User name(s): + + { + setUserFilters(items); + actionMeta.action === 'select-option' && queryLibraryTrackFilterDatasource(); + }} + value={userFilters} + options={userData.map((r) => { + return { value: r, label: r }; + })} + placeholder={'Filter queries for user name(s)'} + aria-label={'Filter queries for user name(s)'} + /> diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/AddedByCell.tsx b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/AddedByCell.tsx index b9903195d47..35793ab7fbd 100644 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/AddedByCell.tsx +++ b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/AddedByCell.tsx @@ -1,14 +1,20 @@ +import { Avatar } from '@grafana/ui'; +import { User } from 'app/features/query-library/types'; + import { useQueryLibraryListStyles } from './styles'; type AddedByCellProps = { - user?: string; + user?: User; }; export function AddedByCell(props: AddedByCellProps) { const styles = useQueryLibraryListStyles(); return (
- {props.user || 'Unknown'} + + + + {props.user?.displayName || 'Unknown'}
); } diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/types.ts b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/types.ts index eae57422d0c..51f5e490953 100644 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/types.ts +++ b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/types.ts @@ -1,4 +1,5 @@ import { DataQuery, DataSourceRef } from '@grafana/schema'; +import { User } from 'app/features/query-library/types'; export type QueryTemplateRow = { index: string; @@ -9,6 +10,6 @@ export type QueryTemplateRow = { datasourceRef?: DataSourceRef | null; datasourceType?: string; createdAtTimestamp?: number; - user?: string; + user?: User; uid?: string; }; diff --git a/public/app/features/explore/QueryLibrary/utils/search.ts b/public/app/features/explore/QueryLibrary/utils/search.ts index 6ad6e277322..2c55be7e9d0 100644 --- a/public/app/features/explore/QueryLibrary/utils/search.ts +++ b/public/app/features/explore/QueryLibrary/utils/search.ts @@ -1,15 +1,23 @@ import { QueryTemplateRow } from '../QueryTemplatesTable/types'; -export const searchQueryLibrary = (queryLibrary: QueryTemplateRow[], query: string, filter: string[]) => { +export const searchQueryLibrary = ( + queryLibrary: QueryTemplateRow[], + query: string, + dsFilters: string[], + userNameFilters: string[] +) => { const result = queryLibrary.filter((item) => { - const matchesFilter = - filter.length === 0 || filter.some((f) => item.datasourceName?.toLowerCase().includes(f.toLowerCase())); + const matchesDsFilter = + dsFilters.length === 0 || dsFilters.some((f) => item.datasourceName?.toLowerCase().includes(f.toLowerCase())); + const matchesUserNameFilter = + userNameFilters.length === 0 || userNameFilters.includes(item.user?.displayName || ''); return ( (item.datasourceName?.toLowerCase().includes(query.toLowerCase()) || item.datasourceType?.toLowerCase().includes(query.toLowerCase()) || item.description?.toLowerCase().includes(query.toLowerCase()) || item.queryText?.toLowerCase().includes(query.toLowerCase())) && - matchesFilter + matchesDsFilter && + matchesUserNameFilter ); }); return result; diff --git a/public/app/features/explore/spec/helper/setup.tsx b/public/app/features/explore/spec/helper/setup.tsx index 0829af1ac70..69ba506324a 100644 --- a/public/app/features/explore/spec/helper/setup.tsx +++ b/public/app/features/explore/spec/helper/setup.tsx @@ -33,7 +33,7 @@ import { GrafanaContext } from 'app/core/context/GrafanaContext'; import { GrafanaRoute } from 'app/core/navigation/GrafanaRoute'; import { Echo } from 'app/core/services/echo/Echo'; import { setLastUsedDatasourceUID } from 'app/core/utils/explore'; -import { QueryLibraryMocks } from 'app/features/query-library'; +import { IdentityServiceMocks, QueryLibraryMocks } from 'app/features/query-library'; import { MIXED_DATASOURCE_NAME } from 'app/plugins/datasource/mixed/MixedDataSource'; import { configureStore } from 'app/store/configureStore'; @@ -77,12 +77,12 @@ export function setupExplore(options?: SetupOptions): { data.totalCount = 0; } else if (req.url.startsWith('/api/query-history') && req.method === 'GET') { data.result = options?.queryHistory || {}; - } else if (req.url.startsWith(QueryLibraryMocks.data.all.url)) { - data = QueryLibraryMocks.data.all.response; + } else if (req.url.startsWith(QueryLibraryMocks.data.url)) { + data = QueryLibraryMocks.data.response; } return of({ data }); }), - get: jest.fn(), + get: jest.fn().mockResolvedValue(IdentityServiceMocks.data.response), patch: jest.fn().mockRejectedValue(undefined), post: jest.fn(), put: jest.fn().mockRejectedValue(undefined), diff --git a/public/app/features/query-library/api/mappers.test.ts b/public/app/features/query-library/api/mappers.test.ts deleted file mode 100644 index 46620ef5166..00000000000 --- a/public/app/features/query-library/api/mappers.test.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { parseCreatedByValue } from './mappers'; - -describe.skip('mappers', () => { - describe('parseCreatedByValue', () => { - it.each` - value | expected - ${''} | ${undefined} - ${'api-key:1'} | ${{ userId: 1 }} - ${'service-account:1:admin'} | ${{ userId: 1, login: 'admin' }} - ${'user:1:admin'} | ${{ userId: 1, login: 'admin' }} - ${'anonymous:0'} | ${undefined} - ${'render:0'} | ${undefined} - ${':0'} | ${undefined} - `("parsing '$value' should be '$expected'", ({ value, expected }) => { - expect(parseCreatedByValue(value)).toEqual(expected); - }); - }); -}); diff --git a/public/app/features/query-library/api/mappers.ts b/public/app/features/query-library/api/mappers.ts index 9dfb2fe4439..a74dd80b1cf 100644 --- a/public/app/features/query-library/api/mappers.ts +++ b/public/app/features/query-library/api/mappers.ts @@ -5,28 +5,6 @@ import { AddQueryTemplateCommand, QueryTemplate } from '../types'; import { API_VERSION, QueryTemplateKinds } from './query'; import { CREATED_BY_KEY, DataQueryFullSpec, DataQuerySpecResponse, DataQueryTarget } from './types'; -export const parseCreatedByValue = (value?: string) => { - // https://github.com/grafana/grafana/blob/main/pkg/services/user/identity.go#L194 - /*if (value !== undefined && value !== '') { - const vals = value.split(':'); - if (vals.length >= 2) { - if (vals[0] === 'anonymous' || vals[0] === 'render' || vals[0] === '') { - return undefined; - } else { - return { - userId: vals[1], - login: vals[2], - }; - } - } else { - return undefined; - } - } else { - return undefined; - }*/ - return !!value ? value : undefined; -}; - export const convertDataQueryResponseToQueryTemplates = (result: DataQuerySpecResponse): QueryTemplate[] => { if (!result.items) { return []; @@ -37,7 +15,9 @@ export const convertDataQueryResponseToQueryTemplates = (result: DataQuerySpecRe title: spec.spec.title, targets: spec.spec.targets.map((target: DataQueryTarget) => target.properties), createdAtTimestamp: new Date(spec.metadata.creationTimestamp || '').getTime(), - user: parseCreatedByValue(spec.metadata?.annotations?.[CREATED_BY_KEY]), + user: { + uid: spec.metadata.annotations?.[CREATED_BY_KEY] || '', + }, }; }); }; diff --git a/public/app/features/query-library/api/mocks.ts b/public/app/features/query-library/api/mocks.ts index 470dae005ae..0fb6d5fe8d5 100644 --- a/public/app/features/query-library/api/mocks.ts +++ b/public/app/features/query-library/api/mocks.ts @@ -1,4 +1,5 @@ import { BASE_URL } from './query'; +import { getIdentityDisplayList } from './testdata/identityDisplayList'; import { getTestQueryList } from './testdata/testQueryList'; export const mockData = { @@ -6,4 +7,7 @@ export const mockData = { url: BASE_URL, response: getTestQueryList(), }, + identityDisplay: { + response: getIdentityDisplayList(), + }, }; diff --git a/public/app/features/query-library/api/testdata/identityDisplayList.ts b/public/app/features/query-library/api/testdata/identityDisplayList.ts new file mode 100644 index 00000000000..5ff6056fa17 --- /dev/null +++ b/public/app/features/query-library/api/testdata/identityDisplayList.ts @@ -0,0 +1,26 @@ +export const getIdentityDisplayList = () => ({ + kind: 'DisplayList', + apiVersion: 'iam.grafana.app/v0alpha1', + metadata: {}, + keys: ['user:u000000001', 'user:u000000002'], + display: [ + { + identity: { + type: 'user', + name: 'u000000001', + }, + displayName: 'Test User1', + avatarURL: '/avatar/46d229b033af06a191ff2267bca9ae56', + internalId: 1, + }, + { + identity: { + type: 'user', + name: 'u000000002', + }, + displayName: 'Test User2', + avatarURL: '/avatar/24c9e15e52afc47c225b757e7bee1f9d', + internalId: 2, + }, + ], +}); diff --git a/public/app/features/query-library/api/testdata/testQueryList.ts b/public/app/features/query-library/api/testdata/testQueryList.ts index 474210ef5d9..c6efa9b93bd 100644 --- a/public/app/features/query-library/api/testdata/testQueryList.ts +++ b/public/app/features/query-library/api/testdata/testQueryList.ts @@ -16,7 +16,7 @@ export const getTestQueryList = () => ({ uid: '65327fce-c545-489d-ada5-16f909453d12', resourceVersion: '1783293341664808960', creationTimestamp: '2024-04-25T20:32:58Z', - annotations: { 'grafana.app/createdBy': 'user:1:admin' }, + annotations: { 'grafana.app/createdBy': 'user:u000000001' }, }, spec: { title: 'Elastic Query Template', @@ -63,7 +63,7 @@ export const getTestQueryList = () => ({ uid: '3e71de65-efa7-40e3-8f23-124212cca455', resourceVersion: '1783214217151647744', creationTimestamp: '2024-04-25T11:05:55Z', - annotations: { 'grafana.app/createdBy': 'user:1:admin' }, + annotations: { 'grafana.app/createdBy': 'user:u000000001' }, }, spec: { title: 'Loki Query Template', diff --git a/public/app/features/query-library/api/types.ts b/public/app/features/query-library/api/types.ts index 5e8eaf542fc..eb42204e7c6 100644 --- a/public/app/features/query-library/api/types.ts +++ b/public/app/features/query-library/api/types.ts @@ -30,4 +30,29 @@ export type DataQuerySpecResponse = { items: DataQueryFullSpec[]; }; +// pkg/apis/iam/v0alpha1/types_display.go +export type UserDataQueryResponse = { + apiVersion: string; + kind: string; + metadata: { + selfLink: string; + resourceVersion: string; + continue: string; + remainingItemCount: number; + }; + display: UserSpecResponse[]; + keys: string[]; +}; + +// pkg/apis/iam/v0alpha1/types_display.go +export type UserSpecResponse = { + avatarUrl: string; + displayName: string; + identity: { + name: string; + type: string; + }; + internalId: number; +}; + export const CREATED_BY_KEY = 'grafana.app/createdBy'; diff --git a/public/app/features/query-library/api/user.ts b/public/app/features/query-library/api/user.ts new file mode 100644 index 00000000000..195fff172af --- /dev/null +++ b/public/app/features/query-library/api/user.ts @@ -0,0 +1,18 @@ +import { getBackendSrv, config } from '@grafana/runtime'; + +import { UserDataQueryResponse } from './types'; + +/** + * @alpha + */ +export const API_VERSION = 'iam.grafana.app/v0alpha1'; + +/** + * @alpha + */ +const BASE_URL = `apis/${API_VERSION}/namespaces/${config.namespace}/display`; + +export async function getUserInfo(url?: string): Promise { + const userInfo = await getBackendSrv().get(`${BASE_URL}${url}`); + return userInfo; +} diff --git a/public/app/features/query-library/index.ts b/public/app/features/query-library/index.ts index 8c5bd02a098..39ab146a3be 100644 --- a/public/app/features/query-library/index.ts +++ b/public/app/features/query-library/index.ts @@ -24,5 +24,9 @@ export function isQueryLibraryEnabled() { } export const QueryLibraryMocks = { - data: mockData, + data: mockData.all, +}; + +export const IdentityServiceMocks = { + data: mockData.identityDisplay, }; diff --git a/public/app/features/query-library/types.ts b/public/app/features/query-library/types.ts index 91e050b2774..22c9f73c57e 100644 --- a/public/app/features/query-library/types.ts +++ b/public/app/features/query-library/types.ts @@ -7,7 +7,7 @@ export type QueryTemplate = { title: string; targets: DataQuery[]; createdAtTimestamp: number; - user?: string; + user?: User; }; export type AddQueryTemplateCommand = { @@ -23,3 +23,9 @@ export type EditQueryTemplateCommand = { export type DeleteQueryTemplateCommand = { uid: string; }; + +export type User = { + uid: string; + displayName?: string; + avatarUrl?: string; +}; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 8a0a1022233..0823f93c9b3 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -2218,7 +2218,9 @@ "datasource-names": "Datasource name(s):", "delete-query-button": "Delete query", "query-template-get-error": "Error attempting to get query template from the library: {{error}}", - "search": "Search by data source, query content or description" + "search": "Search by data source, query content or description", + "user-info-get-error": "Error attempting to get user info from the library: {{error}}", + "user-names": "User name(s):" }, "query-operation": { "header": { diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json index 2a20455c393..a2a87a13fd4 100644 --- a/public/locales/pseudo-LOCALE/grafana.json +++ b/public/locales/pseudo-LOCALE/grafana.json @@ -2218,7 +2218,9 @@ "datasource-names": "Đäŧäşőūřčę ʼnämę(ş):", "delete-query-button": "Đęľęŧę qūęřy", "query-template-get-error": "Ēřřőř äŧŧęmpŧįʼnģ ŧő ģęŧ qūęřy ŧęmpľäŧę ƒřőm ŧĥę ľįþřäřy: {{error}}", - "search": "Ŝęäřčĥ þy đäŧä şőūřčę, qūęřy čőʼnŧęʼnŧ őř đęşčřįpŧįőʼn" + "search": "Ŝęäřčĥ þy đäŧä şőūřčę, qūęřy čőʼnŧęʼnŧ őř đęşčřįpŧįőʼn", + "user-info-get-error": "Ēřřőř äŧŧęmpŧįʼnģ ŧő ģęŧ ūşęř įʼnƒő ƒřőm ŧĥę ľįþřäřy: {{error}}", + "user-names": "Ůşęř ʼnämę(ş):" }, "query-operation": { "header": {