From 932429a5458b19facbc8afcc3aff1c35117559e7 Mon Sep 17 00:00:00 2001 From: Leo <108552997+lpskdl@users.noreply.github.com> Date: Wed, 2 Nov 2022 16:49:02 +0100 Subject: [PATCH] LibraryPanels: Replace folderID with folderUID (#56414) * user essentials mob! :trident: lastFile:pkg/services/libraryelements/writers.go * user essentials mob! :trident: lastFile:pkg/services/libraryelements/writers.go * user essentials mob! :trident: lastFile:pkg/services/libraryelements/writers.go * user essentials mob! :trident: lastFile:pkg/services/libraryelements/writers.go * user essentials mob! :trident: lastFile:pkg/services/libraryelements/database.go * user essentials mob! :trident: lastFile:pkg/services/libraryelements/writers.go * user essentials mob! :trident: lastFile:pkg/services/libraryelements/writers.go * user essentials mob! :trident: * support filterFolderUIDs in the frontend * move common logic to a variable * fixed FolderLibraryPanelsPage and improved unit test * fix backend lint error * fix formatting error Co-authored-by: Joao Silva Co-authored-by: Ashley Harrison Co-authored-by: eledobleefe Co-authored-by: joshhunt --- pkg/services/libraryelements/api.go | 17 ++-- pkg/services/libraryelements/database.go | 1 + pkg/services/libraryelements/guard.go | 5 ++ pkg/services/libraryelements/models.go | 17 ++-- pkg/services/libraryelements/writers.go | 82 +++++++++++++------ .../components/FolderFilter/FolderFilter.tsx | 4 +- .../folders/FolderLibraryPanelsPage.tsx | 5 +- .../LibraryPanelsSearch.test.tsx | 49 +++++++++-- .../LibraryPanelsSearch.tsx | 8 +- .../LibraryPanelsView/LibraryPanelsView.tsx | 2 +- .../components/LibraryPanelsView/actions.ts | 4 +- .../app/features/library-panels/state/api.ts | 6 +- public/app/types/folders.ts | 4 + 13 files changed, 138 insertions(+), 66 deletions(-) diff --git a/pkg/services/libraryelements/api.go b/pkg/services/libraryelements/api.go index 93035920d37..3cb3ef4b5cc 100644 --- a/pkg/services/libraryelements/api.go +++ b/pkg/services/libraryelements/api.go @@ -133,14 +133,15 @@ func (l *LibraryElementService) getHandler(c *models.ReqContext) response.Respon // 500: internalServerError func (l *LibraryElementService) getAllHandler(c *models.ReqContext) response.Response { query := searchLibraryElementsQuery{ - perPage: c.QueryInt("perPage"), - page: c.QueryInt("page"), - searchString: c.Query("searchString"), - sortDirection: c.Query("sortDirection"), - kind: c.QueryInt("kind"), - typeFilter: c.Query("typeFilter"), - excludeUID: c.Query("excludeUid"), - folderFilter: c.Query("folderFilter"), + perPage: c.QueryInt("perPage"), + page: c.QueryInt("page"), + searchString: c.Query("searchString"), + sortDirection: c.Query("sortDirection"), + kind: c.QueryInt("kind"), + typeFilter: c.Query("typeFilter"), + excludeUID: c.Query("excludeUid"), + folderFilter: c.Query("folderFilter"), + folderFilterUIDs: c.Query("folderFilterUIDs"), } elementsResult, err := l.getAllLibraryElements(c.Req.Context(), c.SignedInUser, query) if err != nil { diff --git a/pkg/services/libraryelements/database.go b/pkg/services/libraryelements/database.go index 56d3240fa07..8e59876fd9a 100644 --- a/pkg/services/libraryelements/database.go +++ b/pkg/services/libraryelements/database.go @@ -409,6 +409,7 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI var libraryElements []LibraryElement countBuilder := db.SQLBuilder{} countBuilder.Write("SELECT * FROM library_element AS le") + countBuilder.Write(" INNER JOIN dashboard AS dashboard on le.folder_id = dashboard.id") countBuilder.Write(` WHERE le.org_id=?`, signedInUser.OrgID) writeKindSQL(query, &countBuilder) writeSearchStringSQL(query, l.SQLStore, &countBuilder) diff --git a/pkg/services/libraryelements/guard.go b/pkg/services/libraryelements/guard.go index 7756b9780a1..ed697a4bf33 100644 --- a/pkg/services/libraryelements/guard.go +++ b/pkg/services/libraryelements/guard.go @@ -4,6 +4,7 @@ import ( "context" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/org" @@ -14,6 +15,10 @@ func isGeneralFolder(folderID int64) bool { return folderID == 0 } +func isUIDGeneralFolder(folderUID string) bool { + return folderUID == accesscontrol.GeneralFolderUID +} + func (l *LibraryElementService) requireSupportedElementKind(kindAsInt int64) error { kind := models.LibraryElementKind(kindAsInt) switch kind { diff --git a/pkg/services/libraryelements/models.go b/pkg/services/libraryelements/models.go index 6bb3e35e625..54af6dbd9a0 100644 --- a/pkg/services/libraryelements/models.go +++ b/pkg/services/libraryelements/models.go @@ -207,14 +207,15 @@ type PatchLibraryElementCommand struct { // searchLibraryElementsQuery is the query used for searching for Elements type searchLibraryElementsQuery struct { - perPage int - page int - searchString string - sortDirection string - kind int - typeFilter string - excludeUID string - folderFilter string + perPage int + page int + searchString string + sortDirection string + kind int + typeFilter string + excludeUID string + folderFilter string + folderFilterUIDs string } // LibraryElementResponse is a response struct for LibraryElementDTO. diff --git a/pkg/services/libraryelements/writers.go b/pkg/services/libraryelements/writers.go index 8ab61835c09..87d598525d0 100644 --- a/pkg/services/libraryelements/writers.go +++ b/pkg/services/libraryelements/writers.go @@ -2,6 +2,7 @@ package libraryelements import ( "bytes" + "errors" "strconv" "strings" @@ -72,41 +73,58 @@ func writeExcludeSQL(query searchLibraryElementsQuery, builder *db.SQLBuilder) { type FolderFilter struct { includeGeneralFolder bool folderIDs []string + folderUIDs []string parseError error } func parseFolderFilter(query searchLibraryElementsQuery) FolderFilter { folderIDs := make([]string, 0) - if len(strings.TrimSpace(query.folderFilter)) == 0 { - return FolderFilter{ - includeGeneralFolder: true, - folderIDs: folderIDs, - parseError: nil, - } - } + folderUIDs := make([]string, 0) + hasFolderFilter := len(strings.TrimSpace(query.folderFilter)) > 0 + hasFolderFilterUID := len(strings.TrimSpace(query.folderFilterUIDs)) > 0 - includeGeneralFolder := false - folderIDs = strings.Split(query.folderFilter, ",") - for _, filter := range folderIDs { - folderID, err := strconv.ParseInt(filter, 10, 64) - if err != nil { - return FolderFilter{ - includeGeneralFolder: false, - folderIDs: folderIDs, - parseError: err, - } - } - if isGeneralFolder(folderID) { - includeGeneralFolder = true - break - } - } - - return FolderFilter{ - includeGeneralFolder: includeGeneralFolder, + result := FolderFilter{ + includeGeneralFolder: true, folderIDs: folderIDs, + folderUIDs: folderUIDs, parseError: nil, } + + if hasFolderFilter && hasFolderFilterUID { + result.parseError = errors.New("cannot pass both folderFilter and folderFilterUIDs") + return result + } + + if hasFolderFilter { + result.includeGeneralFolder = false + folderIDs = strings.Split(query.folderFilter, ",") + result.folderIDs = folderIDs + for _, filter := range folderIDs { + folderID, err := strconv.ParseInt(filter, 10, 64) + if err != nil { + result.parseError = err + } + if isGeneralFolder(folderID) { + result.includeGeneralFolder = true + break + } + } + } + + if hasFolderFilterUID { + result.includeGeneralFolder = false + folderUIDs = strings.Split(query.folderFilterUIDs, ",") + result.folderUIDs = folderUIDs + + for _, folderUID := range folderUIDs { + if isUIDGeneralFolder(folderUID) { + result.includeGeneralFolder = true + break + } + } + } + + return result } func (f *FolderFilter) writeFolderFilterSQL(includeGeneral bool, builder *db.SQLBuilder) error { @@ -127,5 +145,17 @@ func (f *FolderFilter) writeFolderFilterSQL(includeGeneral bool, builder *db.SQL builder.Write(sql.String(), params...) } + paramsUIDs := make([]interface{}, 0) + for _, folderUID := range f.folderUIDs { + if !includeGeneral && isUIDGeneralFolder(folderUID) { + continue + } + paramsUIDs = append(paramsUIDs, folderUID) + } + if len(paramsUIDs) > 0 { + sql.WriteString(` AND dashboard.uid IN (?` + strings.Repeat(",?", len(paramsUIDs)-1) + ")") + builder.Write(sql.String(), paramsUIDs...) + } + return nil } diff --git a/public/app/core/components/FolderFilter/FolderFilter.tsx b/public/app/core/components/FolderFilter/FolderFilter.tsx index 8e382fd3421..197b9eadc62 100644 --- a/public/app/core/components/FolderFilter/FolderFilter.tsx +++ b/public/app/core/components/FolderFilter/FolderFilter.tsx @@ -73,9 +73,9 @@ async function getFoldersAsOptions( // FIXME: stop using id from search and use UID instead const searchHits: DashboardSearchHit[] = await getBackendSrv().search(params); - const options = searchHits.map((d) => ({ label: d.title, value: { id: d.id, title: d.title } })); + const options = searchHits.map((d) => ({ label: d.title, value: { uid: d.uid, title: d.title } })); if (!searchString || 'general'.includes(searchString.toLowerCase())) { - options.unshift({ label: 'General', value: { id: 0, title: 'General' } }); + options.unshift({ label: 'General', value: { uid: 'general', title: 'General' } }); } setLoading(false); diff --git a/public/app/features/folders/FolderLibraryPanelsPage.tsx b/public/app/features/folders/FolderLibraryPanelsPage.tsx index 24ae4ab1534..3708da258d0 100644 --- a/public/app/features/folders/FolderLibraryPanelsPage.tsx +++ b/public/app/features/folders/FolderLibraryPanelsPage.tsx @@ -21,7 +21,6 @@ const mapStateToProps = (state: StoreState, props: OwnProps) => { return { pageNav: getNavModel(state.navIndex, `folder-library-panels-${uid}`, getLoadingNav(1)), folderUid: uid, - folder: state.folder, }; }; @@ -33,7 +32,7 @@ const connector = connect(mapStateToProps, mapDispatchToProps); export type Props = OwnProps & ConnectedProps; -export function FolderLibraryPanelsPage({ pageNav, getFolderByUid, folderUid, folder }: Props): JSX.Element { +export function FolderLibraryPanelsPage({ pageNav, getFolderByUid, folderUid }: Props): JSX.Element { const { loading } = useAsync(async () => await getFolderByUid(folderUid), [getFolderByUid, folderUid]); const [selected, setSelected] = useState(undefined); @@ -42,7 +41,7 @@ export function FolderLibraryPanelsPage({ pageNav, getFolderByUid, folderUid, fo { await waitFor(() => expect(getLibraryPanelsSpy).toHaveBeenCalledWith({ searchString: 'a', - folderFilter: [], + folderFilterUIDs: [], page: 0, typeFilter: [], perPage: 40, @@ -128,7 +128,7 @@ describe('LibraryPanelsSearch', () => { expect(getLibraryPanelsSpy).toHaveBeenCalledWith({ searchString: '', sortDirection: 'alpha-desc', - folderFilter: [], + folderFilterUIDs: [], page: 0, typeFilter: [], perPage: 40, @@ -156,7 +156,7 @@ describe('LibraryPanelsSearch', () => { await waitFor(() => expect(getLibraryPanelsSpy).toHaveBeenCalledWith({ searchString: '', - folderFilter: [], + folderFilterUIDs: [], page: 0, typeFilter: ['graph', 'timeseries'], perPage: 40, @@ -177,21 +177,52 @@ describe('LibraryPanelsSearch', () => { describe('and user changes folder filter', () => { it('should call api with correct params', async () => { - const { getLibraryPanelsSpy } = await getTestContext({ showFolderFilter: true }); + const { getLibraryPanelsSpy } = await getTestContext( + { showFolderFilter: true, currentFolderUID: 'wXyZ1234' }, + { + elements: [ + { + id: 1, + name: 'Library Panel Name', + kind: LibraryElementKind.Panel, + uid: 'uid', + description: 'Library Panel Description', + folderId: 0, + model: { type: 'timeseries', title: 'A title' }, + type: 'timeseries', + orgId: 1, + version: 1, + meta: { + folderName: 'General', + folderUid: '', + connectedDashboards: 0, + created: '2021-01-01 12:00:00', + createdBy: { id: 1, name: 'Admin', avatarUrl: '' }, + updated: '2021-01-01 12:00:00', + updatedBy: { id: 1, name: 'Admin', avatarUrl: '' }, + }, + }, + ], + perPage: 40, + page: 1, + totalCount: 0, + } + ); await userEvent.click(screen.getByRole('combobox', { name: /folder filter/i })); - await userEvent.type(screen.getByRole('combobox', { name: /folder filter/i }), '{enter}', { + await userEvent.type(screen.getByRole('combobox', { name: /folder filter/i }), 'library', { skipClick: true, }); - await waitFor(() => + + await waitFor(() => { expect(getLibraryPanelsSpy).toHaveBeenCalledWith({ searchString: '', - folderFilter: ['0'], + folderFilterUIDs: ['wXyZ1234'], page: 0, typeFilter: [], perPage: 40, - }) - ); + }); + }); }); }); }); diff --git a/public/app/features/library-panels/components/LibraryPanelsSearch/LibraryPanelsSearch.tsx b/public/app/features/library-panels/components/LibraryPanelsSearch/LibraryPanelsSearch.tsx index d77cc42c49d..aedb7d918d6 100644 --- a/public/app/features/library-panels/components/LibraryPanelsSearch/LibraryPanelsSearch.tsx +++ b/public/app/features/library-panels/components/LibraryPanelsSearch/LibraryPanelsSearch.tsx @@ -26,7 +26,7 @@ export interface LibraryPanelsSearchProps { showFolderFilter?: boolean; showSecondaryActions?: boolean; currentPanelId?: string; - currentFolderId?: number; + currentFolderUID?: string; perPage?: number; } @@ -34,7 +34,7 @@ export const LibraryPanelsSearch = ({ onClick, variant = LibraryPanelsSearchVariant.Spacious, currentPanelId, - currentFolderId, + currentFolderUID, perPage = DEFAULT_PER_PAGE_PAGINATION, showPanelFilter = false, showFolderFilter = false, @@ -48,7 +48,7 @@ export const LibraryPanelsSearch = ({ useDebounce(() => setDebouncedSearchQuery(searchQuery), 200, [searchQuery]); const [sortDirection, setSortDirection] = useState>({}); - const [folderFilter, setFolderFilter] = useState(currentFolderId ? [String(currentFolderId)] : []); + const [folderFilter, setFolderFilter] = useState(currentFolderUID ? [currentFolderUID] : []); const [panelFilter, setPanelFilter] = useState([]); const sortOrFiltersVisible = showSort || showPanelFilter || showFolderFilter; @@ -155,7 +155,7 @@ const SearchControls = React.memo( [onPanelFilterChange] ); const folderFilterChanged = useCallback( - (folders: FolderInfo[]) => onFolderFilterChange(folders.map((f) => String(f.id))), + (folders: FolderInfo[]) => onFolderFilterChange(folders.map((f) => f.uid ?? '')), [onFolderFilterChange] ); diff --git a/public/app/features/library-panels/components/LibraryPanelsView/LibraryPanelsView.tsx b/public/app/features/library-panels/components/LibraryPanelsView/LibraryPanelsView.tsx index c3b7c9dd866..39431a8c3d3 100644 --- a/public/app/features/library-panels/components/LibraryPanelsView/LibraryPanelsView.tsx +++ b/public/app/features/library-panels/components/LibraryPanelsView/LibraryPanelsView.tsx @@ -51,7 +51,7 @@ export const LibraryPanelsView: React.FC = ({ searchString, sortDirection, panelFilter, - folderFilter, + folderFilterUIDs: folderFilter, page, perPage, currentPanelId, diff --git a/public/app/features/library-panels/components/LibraryPanelsView/actions.ts b/public/app/features/library-panels/components/LibraryPanelsView/actions.ts index f304c318f22..ada907caaf0 100644 --- a/public/app/features/library-panels/components/LibraryPanelsView/actions.ts +++ b/public/app/features/library-panels/components/LibraryPanelsView/actions.ts @@ -14,7 +14,7 @@ interface SearchArgs { searchString: string; sortDirection?: string; panelFilter?: string[]; - folderFilter?: string[]; + folderFilterUIDs?: string[]; currentPanelId?: string; } @@ -29,7 +29,7 @@ export function searchForLibraryPanels(args: SearchArgs): DispatchResult { excludeUid: args.currentPanelId, sortDirection: args.sortDirection, typeFilter: args.panelFilter, - folderFilter: args.folderFilter, + folderFilterUIDs: args.folderFilterUIDs, }) ).pipe( mergeMap(({ perPage, elements: libraryPanels, page, totalCount }) => diff --git a/public/app/features/library-panels/state/api.ts b/public/app/features/library-panels/state/api.ts index 04fe91d3706..2462a6f88a1 100644 --- a/public/app/features/library-panels/state/api.ts +++ b/public/app/features/library-panels/state/api.ts @@ -19,7 +19,7 @@ export interface GetLibraryPanelsOptions { excludeUid?: string; sortDirection?: string; typeFilter?: string[]; - folderFilter?: string[]; + folderFilterUIDs?: string[]; } export async function getLibraryPanels({ @@ -29,13 +29,13 @@ export async function getLibraryPanels({ excludeUid = '', sortDirection = '', typeFilter = [], - folderFilter = [], + folderFilterUIDs = [], }: GetLibraryPanelsOptions = {}): Promise { const params = new URLSearchParams(); params.append('searchString', searchString); params.append('sortDirection', sortDirection); params.append('typeFilter', typeFilter.join(',')); - params.append('folderFilter', folderFilter.join(',')); + params.append('folderFilterUIDs', folderFilterUIDs.join(',')); params.append('excludeUid', excludeUid); params.append('perPage', perPage.toString(10)); params.append('page', page.toString(10)); diff --git a/public/app/types/folders.ts b/public/app/types/folders.ts index 21a1fb2da78..bd0616499bf 100644 --- a/public/app/types/folders.ts +++ b/public/app/types/folders.ts @@ -28,7 +28,11 @@ export interface FolderState { } export interface FolderInfo { + /** + * @deprecated use uid instead. + */ id?: number; + uid?: string; title?: string; url?: string; canViewFolderPermissions?: boolean;