From 7e8612f0da26817164d2fc05b008eb2efbec4605 Mon Sep 17 00:00:00 2001 From: "Qingyang(Abby) Hu" Date: Wed, 4 Sep 2024 16:43:10 -0700 Subject: [PATCH 01/12] Add query result and time to the query editor footer (#7951) * add query result to the footer Signed-off-by: abbyhu2000 * fix unit test Signed-off-by: abbyhu2000 * conditional language Signed-off-by: abbyhu2000 * native language still use toast Signed-off-by: abbyhu2000 * refactor to not use two states Signed-off-by: abbyhu2000 * address comments Signed-off-by: abbyhu2000 --------- Signed-off-by: abbyhu2000 --- changelogs/fragments/7951.yml | 2 + src/plugins/data/public/index.ts | 2 + .../data/public/query/query_string/index.ts | 2 + .../query_string/language_service/index.ts | 2 +- .../language_service/lib/index.ts | 1 + .../language_service/lib/query_result.tsx | 92 +++++++++++++++++++ .../query_string/query_string_manager.ts | 1 + .../public/ui/query_editor/query_editor.tsx | 5 + .../ui/query_editor/query_editor_top_row.tsx | 4 +- .../ui/search_bar/create_search_bar.tsx | 1 + .../data/public/ui/search_bar/search_bar.tsx | 4 +- src/plugins/data/public/ui/types.ts | 1 + src/plugins/data/public/ui/ui_service.ts | 3 + .../view_components/canvas/index.tsx | 3 + .../view_components/canvas/top_nav.tsx | 21 ++++- .../application/view_components/index.ts | 1 + .../view_components/utils/index.tsx | 6 ++ .../view_components/utils/use_search.ts | 45 ++++++++- src/plugins/discover/public/index.ts | 1 + .../adapters/request/request_responder.ts | 4 + .../public/top_nav_menu/top_nav_menu.tsx | 3 + .../query_enhancements/common/utils.ts | 17 +--- .../public/search/ppl_search_interceptor.ts | 9 +- .../query_enhancements/server/routes/index.ts | 5 +- .../server/search/sql_search_strategy.test.ts | 22 ++--- .../server/search/sql_search_strategy.ts | 9 +- 26 files changed, 215 insertions(+), 51 deletions(-) create mode 100644 changelogs/fragments/7951.yml create mode 100644 src/plugins/data/public/query/query_string/language_service/lib/query_result.tsx create mode 100644 src/plugins/discover/public/application/view_components/utils/index.tsx diff --git a/changelogs/fragments/7951.yml b/changelogs/fragments/7951.yml new file mode 100644 index 000000000000..c4883e336671 --- /dev/null +++ b/changelogs/fragments/7951.yml @@ -0,0 +1,2 @@ +feat: +- Add query result and time to the query editor footer ([#7951](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7951)) \ No newline at end of file diff --git a/src/plugins/data/public/index.ts b/src/plugins/data/public/index.ts index 0ca84c184bf4..cdf4d29697ca 100644 --- a/src/plugins/data/public/index.ts +++ b/src/plugins/data/public/index.ts @@ -473,6 +473,8 @@ export { LanguageServiceContract, RecentQueriesTable, QueryControls, + QueryResult, + QueryStatus, SavedQuery, SavedQueryService, SavedQueryTimeFilter, diff --git a/src/plugins/data/public/query/query_string/index.ts b/src/plugins/data/public/query/query_string/index.ts index c8f6df921ad3..a004103e971b 100644 --- a/src/plugins/data/public/query/query_string/index.ts +++ b/src/plugins/data/public/query/query_string/index.ts @@ -37,4 +37,6 @@ export { EditorEnhancements, RecentQueriesTable, QueryControls, + QueryResult, + QueryStatus, } from './language_service'; diff --git a/src/plugins/data/public/query/query_string/language_service/index.ts b/src/plugins/data/public/query/query_string/language_service/index.ts index cd04fcb50724..70df0971f50c 100644 --- a/src/plugins/data/public/query/query_string/language_service/index.ts +++ b/src/plugins/data/public/query/query_string/language_service/index.ts @@ -5,4 +5,4 @@ export * from './types'; export { LanguageServiceContract, LanguageService } from './language_service'; -export { RecentQueriesTable, QueryControls } from './lib'; +export { RecentQueriesTable, QueryControls, QueryResult, QueryStatus } from './lib'; diff --git a/src/plugins/data/public/query/query_string/language_service/lib/index.ts b/src/plugins/data/public/query/query_string/language_service/lib/index.ts index ca93870dfd4d..dbaaec3e1905 100644 --- a/src/plugins/data/public/query/query_string/language_service/lib/index.ts +++ b/src/plugins/data/public/query/query_string/language_service/lib/index.ts @@ -9,3 +9,4 @@ export * from './lucene_language'; export * from './default_language_reference'; export * from './get_query_control_links'; export * from './recent_query'; +export * from './query_result'; diff --git a/src/plugins/data/public/query/query_string/language_service/lib/query_result.tsx b/src/plugins/data/public/query/query_string/language_service/lib/query_result.tsx new file mode 100644 index 000000000000..9806b7cd55af --- /dev/null +++ b/src/plugins/data/public/query/query_string/language_service/lib/query_result.tsx @@ -0,0 +1,92 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { i18n } from '@osd/i18n'; + +import './_recent_query.scss'; +import { EuiButtonEmpty, EuiPopover, EuiText, EuiPopoverTitle } from '@elastic/eui'; + +import React, { useState } from 'react'; + +export enum ResultStatus { + UNINITIALIZED = 'uninitialized', + LOADING = 'loading', // initial data load + READY = 'ready', // results came back + NO_RESULTS = 'none', // no results came back + ERROR = 'error', // error occurred +} + +export interface QueryStatus { + status: ResultStatus; + body?: { + error?: { + reason?: string; + details: string; + }; + statusCode?: number; + }; + elapsedMs?: number; +} + +export function QueryResult(props: { queryStatus: QueryStatus }) { + const [isPopoverOpen, setPopover] = useState(false); + const onButtonClick = () => { + setPopover(!isPopoverOpen); + }; + + if (props.queryStatus.status === ResultStatus.READY) { + return ( + {}}> + + {props.queryStatus.elapsedMs + ? `Completed in ${props.queryStatus.elapsedMs} ms` + : 'Completed'} + + + ); + } + + if (!props.queryStatus.body || !props.queryStatus.body.error) { + return null; + } + + return ( + + + {'Error'} + + + } + isOpen={isPopoverOpen} + closePopover={() => setPopover(false)} + panelPaddingSize="s" + anchorPosition={'downRight'} + > + ERRORS +
+ + + {i18n.translate('data.query.languageService.queryResults.reasons', { + defaultMessage: `Reasons:`, + })} + + {props.queryStatus.body.error.reason} + + +

+ + {i18n.translate('data.query.languageService.queryResults.details', { + defaultMessage: `Details:`, + })} + {' '} + {props.queryStatus.body.error.details} +

+ +
+ + ); +} diff --git a/src/plugins/data/public/query/query_string/query_string_manager.ts b/src/plugins/data/public/query/query_string/query_string_manager.ts index e26f3a042c03..3fc9a53deff3 100644 --- a/src/plugins/data/public/query/query_string/query_string_manager.ts +++ b/src/plugins/data/public/query/query_string/query_string_manager.ts @@ -166,6 +166,7 @@ export class QueryStringManager { const language = this.languageService.getLanguage(languageId); const dataset = curQuery.dataset; const input = language?.getQueryString(curQuery) || ''; + this.languageService.setUserQueryString(input); return { query: input, diff --git a/src/plugins/data/public/ui/query_editor/query_editor.tsx b/src/plugins/data/public/ui/query_editor/query_editor.tsx index 6ef8a256e21d..dc75d0be3d7f 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor.tsx @@ -14,6 +14,7 @@ import { EuiText, PopoverAnchorPosition, } from '@elastic/eui'; +import { BehaviorSubject } from 'rxjs'; import classNames from 'classnames'; import { isEqual } from 'lodash'; import React, { Component, createRef, RefObject } from 'react'; @@ -27,6 +28,8 @@ import { TimeRange, QueryControls, RecentQueriesTable, + QueryResult, + QueryStatus, } from '../..'; import { OpenSearchDashboardsReactContextValue } from '../../../../opensearch_dashboards_react/public'; import { fromUser, getQueryLog, PersistedLog, toUser } from '../../query'; @@ -61,6 +64,7 @@ export interface QueryEditorProps { filterBar?: any; prepend?: React.ComponentProps['prepend']; savedQueryManagement?: any; + queryStatus?: QueryStatus; } interface Props extends QueryEditorProps { @@ -366,6 +370,7 @@ export default class QueryEditorUI extends Component { {this.props.query.dataset?.timeFieldName || ''} , + , ], end: [ ; savedQueryManagement?: any; + queryStatus?: QueryStatus; } // Needed for React.lazy @@ -186,6 +187,7 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) { dataTestSubj={props.dataTestSubj} filterBar={props.filterBar} savedQueryManagement={props.savedQueryManagement} + queryStatus={props.queryStatus} /> ); diff --git a/src/plugins/data/public/ui/search_bar/create_search_bar.tsx b/src/plugins/data/public/ui/search_bar/create_search_bar.tsx index b3b240dfa2f1..03bb2abfa508 100644 --- a/src/plugins/data/public/ui/search_bar/create_search_bar.tsx +++ b/src/plugins/data/public/ui/search_bar/create_search_bar.tsx @@ -191,6 +191,7 @@ export function createSearchBar({ core, storage, data }: StatefulSearchBarDeps) showQueryBar={props.showQueryBar} showQueryInput={props.showQueryInput} showSaveQuery={props.showSaveQuery} + queryStatus={props.queryStatus} screenTitle={props.screenTitle} indexPatterns={props.indexPatterns} indicateNoData={props.indicateNoData} diff --git a/src/plugins/data/public/ui/search_bar/search_bar.tsx b/src/plugins/data/public/ui/search_bar/search_bar.tsx index bb9a2c7eb28c..975b0535c9d8 100644 --- a/src/plugins/data/public/ui/search_bar/search_bar.tsx +++ b/src/plugins/data/public/ui/search_bar/search_bar.tsx @@ -38,7 +38,7 @@ import { withOpenSearchDashboards, } from '../../../../opensearch_dashboards_react/public'; import { Filter, IIndexPattern, Query, TimeRange, UI_SETTINGS } from '../../../common'; -import { SavedQuery, SavedQueryAttributes, TimeHistoryContract } from '../../query'; +import { SavedQuery, SavedQueryAttributes, TimeHistoryContract, QueryStatus } from '../../query'; import { IDataPluginServices } from '../../types'; import { FilterBar } from '../filter_bar/filter_bar'; import { QueryEditorTopRow } from '../query_editor'; @@ -92,6 +92,7 @@ export interface SearchBarOwnProps { onRefresh?: (payload: { dateRange: TimeRange }) => void; indicateNoData?: boolean; + queryStatus?: QueryStatus; } export type SearchBarProps = SearchBarOwnProps & SearchBarInjectedDeps; @@ -550,6 +551,7 @@ class SearchBarUI extends Component { indicateNoData={this.props.indicateNoData} datePickerRef={this.props.datePickerRef} savedQueryManagement={searchBarMenu(false, true)} + queryStatus={this.props.queryStatus} /> ); } diff --git a/src/plugins/data/public/ui/types.ts b/src/plugins/data/public/ui/types.ts index ec57cb2e8c2c..d9f656fabb03 100644 --- a/src/plugins/data/public/ui/types.ts +++ b/src/plugins/data/public/ui/types.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { QueryStatus } from '../query'; import { IndexPatternSelectProps } from './index_pattern_select'; import { StatefulSearchBarProps } from './search_bar'; import { SuggestionsComponentProps } from './typeahead/suggestions_component'; diff --git a/src/plugins/data/public/ui/ui_service.ts b/src/plugins/data/public/ui/ui_service.ts index 87cfcf630965..4eb45b1a67f2 100644 --- a/src/plugins/data/public/ui/ui_service.ts +++ b/src/plugins/data/public/ui/ui_service.ts @@ -4,6 +4,7 @@ */ import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from 'src/core/public'; +import { BehaviorSubject } from 'rxjs'; import { ConfigSchema } from '../../config'; import { DataPublicPluginStart } from '../types'; import { createIndexPatternSelect } from './index_pattern_select'; @@ -11,6 +12,8 @@ import { createSearchBar } from './search_bar/create_search_bar'; import { SuggestionsComponent } from './typeahead'; import { IUiSetup, IUiStart } from './types'; import { DataStorage } from '../../common'; +import { QueryStatus } from '../query'; +import { ResultStatus } from '../query/query_string/language_service/lib'; /** @internal */ // eslint-disable-next-line @typescript-eslint/no-empty-interface diff --git a/src/plugins/discover/public/application/view_components/canvas/index.tsx b/src/plugins/discover/public/application/view_components/canvas/index.tsx index a44ac89c5d62..2e956d6908e9 100644 --- a/src/plugins/discover/public/application/view_components/canvas/index.tsx +++ b/src/plugins/discover/public/application/view_components/canvas/index.tsx @@ -144,6 +144,9 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history, optionalR {fetchState.status === ResultStatus.NO_RESULTS && ( )} + {fetchState.status === ResultStatus.ERROR && ( + + )} {fetchState.status === ResultStatus.UNINITIALIZED && ( refetch$.next()} /> )} diff --git a/src/plugins/discover/public/application/view_components/canvas/top_nav.tsx b/src/plugins/discover/public/application/view_components/canvas/top_nav.tsx index 6cccc06cc0ee..48b6da5a39a5 100644 --- a/src/plugins/discover/public/application/view_components/canvas/top_nav.tsx +++ b/src/plugins/discover/public/application/view_components/canvas/top_nav.tsx @@ -9,7 +9,11 @@ import { createPortal } from 'react-dom'; import { EuiButtonIcon, EuiFlexGroup, EuiFlexItem, EuiToolTip } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { AppMountParameters } from '../../../../../../core/public'; -import { connectStorageToQueryState, opensearchFilters } from '../../../../../data/public'; +import { + connectStorageToQueryState, + opensearchFilters, + QueryStatus, +} from '../../../../../data/public'; import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public'; import { PLUGIN_ID } from '../../../../common'; import { DiscoverViewServices } from '../../../build_services'; @@ -21,6 +25,7 @@ import { useDispatch, setSavedQuery, useSelector } from '../../utils/state_manag import './discover_canvas.scss'; import { TopNavMenuItemRenderType } from '../../../../../navigation/public'; +import { ResultStatus } from '../utils'; export interface TopNavProps { opts: { @@ -34,9 +39,10 @@ export interface TopNavProps { export const TopNav = ({ opts, showSaveQuery, isEnhancementsEnabled }: TopNavProps) => { const { services } = useOpenSearchDashboards(); - const { inspectorAdapters, savedSearch, indexPattern } = useDiscoverContext(); + const { data$, inspectorAdapters, savedSearch, indexPattern } = useDiscoverContext(); const [indexPatterns, setIndexPatterns] = useState(undefined); const [screenTitle, setScreenTitle] = useState(''); + const [queryStatus, setQueryStatus] = useState({ status: ResultStatus.READY }); const state = useSelector((s) => s.discover); const dispatch = useDispatch(); @@ -69,6 +75,16 @@ export const TopNav = ({ opts, showSaveQuery, isEnhancementsEnabled }: TopNavPro uiSettings ); + useEffect(() => { + const subscription = data$.subscribe((queryData) => { + const result = { + status: queryData.status, + ...queryData.queryStatus, + }; + setQueryStatus(result); + }); + }, [data$]); + useEffect(() => { let isMounted = true; const initializeDataset = async () => { @@ -160,6 +176,7 @@ export const TopNav = ({ opts, showSaveQuery, isEnhancementsEnabled }: TopNavPro datePickerRef={opts?.optionalRef?.datePickerRef} groupActions={showActionsInGroup} screenTitle={screenTitle} + queryStatus={queryStatus} /> ); diff --git a/src/plugins/discover/public/application/view_components/index.ts b/src/plugins/discover/public/application/view_components/index.ts index 45fd68cf1285..7aff16330f66 100644 --- a/src/plugins/discover/public/application/view_components/index.ts +++ b/src/plugins/discover/public/application/view_components/index.ts @@ -5,3 +5,4 @@ export * from './canvas'; export * from './panel'; +export * from './utils'; diff --git a/src/plugins/discover/public/application/view_components/utils/index.tsx b/src/plugins/discover/public/application/view_components/utils/index.tsx new file mode 100644 index 000000000000..c9dbbb829a3f --- /dev/null +++ b/src/plugins/discover/public/application/view_components/utils/index.tsx @@ -0,0 +1,6 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +export { SearchData, ResultStatus } from './use_search'; diff --git a/src/plugins/discover/public/application/view_components/utils/use_search.ts b/src/plugins/discover/public/application/view_components/utils/use_search.ts index b6c13d4982f2..ccb22f86bb29 100644 --- a/src/plugins/discover/public/application/view_components/utils/use_search.ts +++ b/src/plugins/discover/public/application/view_components/utils/use_search.ts @@ -12,7 +12,7 @@ import { cloneDeep } from 'lodash'; import { useLocation } from 'react-router-dom'; import { RequestAdapter } from '../../../../../inspector/public'; import { DiscoverViewServices } from '../../../build_services'; -import { search } from '../../../../../data/public'; +import { QueryStatus, search } from '../../../../../data/public'; import { validateTimeRange } from '../../helpers/validate_time_range'; import { updateSearchSource } from './update_search_source'; import { useIndexPattern } from './use_index_pattern'; @@ -39,6 +39,7 @@ export enum ResultStatus { LOADING = 'loading', // initial data load READY = 'ready', // results came back NO_RESULTS = 'none', // no results came back + ERROR = 'error', // error occurred } export interface SearchData { @@ -50,6 +51,16 @@ export interface SearchData { bucketInterval?: TimechartHeaderBucketInterval | {}; chartData?: Chart; title?: string; + queryStatus?: { + body?: { + error?: { + reason?: string; + details: string; + }; + statusCode?: number; + }; + elapsedMs?: number; + }; } export type SearchRefetch = 'refetch' | undefined; @@ -149,6 +160,8 @@ export const useSearch = (services: DiscoverViewServices) => { dataset = searchSource.getField('index'); + let elapsedMs; + try { // Only show loading indicator if we are fetching when the rows are empty if (fetchStateRef.current.rows?.length === 0) { @@ -180,6 +193,7 @@ export const useSearch = (services: DiscoverViewServices) => { .ok({ json: fetchResp }); const hits = fetchResp.hits.total as number; const rows = fetchResp.hits.hits; + elapsedMs = inspectorRequest.getTime(); let bucketInterval = {}; let chartData; for (const row of rows) { @@ -216,17 +230,38 @@ export const useSearch = (services: DiscoverViewServices) => { indexPattern?.title !== searchSource.getDataFrame()?.name ? searchSource.getDataFrame()?.name : indexPattern?.title, + queryStatus: { + elapsedMs, + }, }); } catch (error) { // If the request was aborted then no need to surface this error in the UI if (error instanceof Error && error.name === 'AbortError') return; + const queryLanguage = data.query.queryString.getQuery().language; + if (queryLanguage === 'kuery' || queryLanguage === 'lucene') { + data$.next({ + status: ResultStatus.NO_RESULTS, + rows: [], + }); + + data.search.showError(error as Error); + return; + } + let errorBody; + try { + errorBody = JSON.parse(error.body.message); + } catch (e) { + errorBody = error.body.message; + } + data$.next({ - status: ResultStatus.NO_RESULTS, - rows: [], + status: ResultStatus.ERROR, + queryStatus: { + body: errorBody, + elapsedMs, + }, }); - - data.search.showError(error as Error); } finally { initalSearchComplete.current = true; } diff --git a/src/plugins/discover/public/index.ts b/src/plugins/discover/public/index.ts index 164aea1fb5bc..54575735a518 100644 --- a/src/plugins/discover/public/index.ts +++ b/src/plugins/discover/public/index.ts @@ -40,3 +40,4 @@ export { SavedSearch, SavedSearchLoader, createSavedSearchesLoader } from './sav export { ISearchEmbeddable, SEARCH_EMBEDDABLE_TYPE, SearchInput } from './embeddable'; export { DISCOVER_APP_URL_GENERATOR, DiscoverUrlGeneratorState } from './url_generator'; +export { SearchData, ResultStatus } from './application/view_components'; diff --git a/src/plugins/inspector/common/adapters/request/request_responder.ts b/src/plugins/inspector/common/adapters/request/request_responder.ts index a65d1b9bc9cd..539257ffaaeb 100644 --- a/src/plugins/inspector/common/adapters/request/request_responder.ts +++ b/src/plugins/inspector/common/adapters/request/request_responder.ts @@ -86,4 +86,8 @@ export class RequestResponder { public error(response: Response): void { this.finish(RequestStatus.ERROR, response); } + + public getTime() { + return this.request.time; + } } diff --git a/src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx b/src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx index e1d93a560e53..7eb32c2a4a6b 100644 --- a/src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx +++ b/src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx @@ -35,6 +35,7 @@ import React, { ReactElement, useRef } from 'react'; import { MountPoint } from '../../../../core/public'; import { DataPublicPluginStart, + QueryStatus, SearchBarProps, StatefulSearchBarProps, } from '../../../data/public'; @@ -82,6 +83,7 @@ export type TopNavMenuProps = Omit & * ``` */ setMenuMountPoint?: (menuMount: MountPoint | undefined) => void; + queryStatus?: QueryStatus; }; /* @@ -157,6 +159,7 @@ export function TopNavMenu(props: TopNavMenuProps): ReactElement | null { {...searchBarProps} showDatePicker={![TopNavMenuItemRenderType.OMITTED, false].includes(showDatePicker!)} {...overrides} + queryStatus={props.queryStatus} /> ); } diff --git a/src/plugins/query_enhancements/common/utils.ts b/src/plugins/query_enhancements/common/utils.ts index 597bf3124496..7388d6beea40 100644 --- a/src/plugins/query_enhancements/common/utils.ts +++ b/src/plugins/query_enhancements/common/utils.ts @@ -5,7 +5,7 @@ import { Query } from 'src/plugins/data/common'; import { from, throwError, timer } from 'rxjs'; -import { filter, mergeMap, take, takeWhile, tap } from 'rxjs/operators'; +import { filter, mergeMap, take, takeWhile } from 'rxjs/operators'; import { EnhancedFetchContext, QueryAggConfig, @@ -42,16 +42,9 @@ export const removeKeyword = (queryString: string | undefined) => { }; export const handleFacetError = (response: any) => { - const error = new Error(response.data); - error.name = response.status; - return throwError(error); -}; - -export const handleFetchError = (response: any) => { - if (response.body.error) { - const error = new Error(response.body.error.response); - return throwError(error); - } + const error = new Error(response.data.body ?? response.data); + error.name = response.data.status ?? response.status; + throw error; }; export const fetch = (context: EnhancedFetchContext, query: Query, aggConfig?: QueryAggConfig) => { @@ -64,7 +57,7 @@ export const fetch = (context: EnhancedFetchContext, query: Query, aggConfig?: Q body, signal, }) - ).pipe(tap(handleFetchError)); + ); }; export const handleQueryStatus = (options: QueryStatusOptions): Promise => { diff --git a/src/plugins/query_enhancements/public/search/ppl_search_interceptor.ts b/src/plugins/query_enhancements/public/search/ppl_search_interceptor.ts index 4618945a35f7..c8a5165cb565 100644 --- a/src/plugins/query_enhancements/public/search/ppl_search_interceptor.ts +++ b/src/plugins/query_enhancements/public/search/ppl_search_interceptor.ts @@ -4,8 +4,7 @@ */ import { trimEnd } from 'lodash'; -import { Observable, throwError } from 'rxjs'; -import { catchError } from 'rxjs/operators'; +import { Observable } from 'rxjs'; import { formatTimePickerDate, Query } from '../../../data/common'; import { DataPublicPluginStart, @@ -52,11 +51,7 @@ export class PPLSearchInterceptor extends SearchInterceptor { const query = this.buildQuery(); - return fetch(context, query, this.getAggConfig(searchRequest, query)).pipe( - catchError((error) => { - return throwError(error); - }) - ); + return fetch(context, query, this.getAggConfig(searchRequest, query)); } public search(request: IOpenSearchDashboardsSearchRequest, options: ISearchOptions) { diff --git a/src/plugins/query_enhancements/server/routes/index.ts b/src/plugins/query_enhancements/server/routes/index.ts index a3673946114d..50bfa30f70ee 100644 --- a/src/plugins/query_enhancements/server/routes/index.ts +++ b/src/plugins/query_enhancements/server/routes/index.ts @@ -85,10 +85,9 @@ function defineRoute( ); return res.ok({ body: { ...queryRes } }); } catch (err) { - logger.error(err); return res.custom({ - statusCode: 500, - body: err, + statusCode: err.name, + body: err.message, }); } } diff --git a/src/plugins/query_enhancements/server/search/sql_search_strategy.test.ts b/src/plugins/query_enhancements/server/search/sql_search_strategy.test.ts index 2df810f7d4b5..234e972e050d 100644 --- a/src/plugins/query_enhancements/server/search/sql_search_strategy.test.ts +++ b/src/plugins/query_enhancements/server/search/sql_search_strategy.test.ts @@ -113,19 +113,15 @@ describe('sqlSearchStrategyProvider', () => { jest.spyOn(facet, 'Facet').mockImplementation(() => mockFacet); const strategy = sqlSearchStrategyProvider(config$, logger, client, usage); - const result = await strategy.search( - emptyRequestHandlerContext, - ({ - body: { query: { query: 'SELECT * FROM table' } }, - } as unknown) as IOpenSearchDashboardsSearchRequest, - {} - ); - - expect(result).toEqual({ - type: DATA_FRAME_TYPES.ERROR, - body: { error: { cause: 'Query failed' } }, - took: 50, - } as IDataFrameError); + await expect( + strategy.search( + emptyRequestHandlerContext, + ({ + body: { query: { query: 'SELECT * FROM table' } }, + } as unknown) as IOpenSearchDashboardsSearchRequest, + {} + ) + ).rejects.toThrow(); }); it('should handle exceptions', async () => { diff --git a/src/plugins/query_enhancements/server/search/sql_search_strategy.ts b/src/plugins/query_enhancements/server/search/sql_search_strategy.ts index ffc7f0a6f0cd..31b03941af24 100644 --- a/src/plugins/query_enhancements/server/search/sql_search_strategy.ts +++ b/src/plugins/query_enhancements/server/search/sql_search_strategy.ts @@ -8,7 +8,6 @@ import { Observable } from 'rxjs'; import { ISearchStrategy, SearchUsage } from '../../../data/server'; import { DATA_FRAME_TYPES, - IDataFrameError, IDataFrameResponse, IOpenSearchDashboardsSearchRequest, Query, @@ -38,11 +37,9 @@ export const sqlSearchStrategyProvider = ( const rawResponse: any = await sqlFacet.describeQuery(context, request); if (!rawResponse.success) { - return { - type: DATA_FRAME_TYPES.ERROR, - body: { error: rawResponse.data }, - took: rawResponse.took, - } as IDataFrameError; + const error = new Error(rawResponse.data.body); + error.name = rawResponse.data.status; + throw error; } const dataFrame = createDataFrame({ From 84d5de3073289686200dfb5acbf177dccff8377a Mon Sep 17 00:00:00 2001 From: Jincheng Wan <45655760+Kapian1234@users.noreply.github.com> Date: Thu, 5 Sep 2024 11:20:34 +0800 Subject: [PATCH 02/12] [Workspace] Hide delete button for non OSD admin (#7987) * Hide workspace delete button for non OSD admin Signed-off-by: Kapian1234 * hide delete action instead of disabled Signed-off-by: Kapian1234 * Changeset file for PR #7987 created/updated --------- Signed-off-by: Kapian1234 Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/7987.yml | 2 ++ .../workspace_detail/workspace_detail.tsx | 35 ++++++++++--------- .../components/workspace_list/index.tsx | 1 + 3 files changed, 22 insertions(+), 16 deletions(-) create mode 100644 changelogs/fragments/7987.yml diff --git a/changelogs/fragments/7987.yml b/changelogs/fragments/7987.yml new file mode 100644 index 000000000000..0b67c17de12d --- /dev/null +++ b/changelogs/fragments/7987.yml @@ -0,0 +1,2 @@ +fix: +- Hide delete button for non OSD admin ([#7987](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7987)) \ No newline at end of file diff --git a/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx b/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx index 3543795f7e44..3d4b77c20a22 100644 --- a/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx +++ b/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx @@ -170,22 +170,25 @@ export const WorkspaceDetail = (props: WorkspaceDetailProps) => { setMountPoint={application.setAppDescriptionControls} /> )} - setDeletedWorkspace(currentWorkspace), - color: 'danger', - iconType: 'trash', - ariaLabel: i18n.translate('workspace.detail.delete.button', { - defaultMessage: 'Delete', - }), - testId: 'workspace-detail-delete-button', - controlType: 'icon', - display: 'base', - } as TopNavControlIconData, - ]} - setMountPoint={application.setAppRightControls} - /> + {isDashboardAdmin && ( + setDeletedWorkspace(currentWorkspace), + color: 'danger', + iconType: 'trash', + ariaLabel: i18n.translate('workspace.detail.delete.button', { + defaultMessage: 'Delete', + }), + testId: 'workspace-detail-delete-button', + controlType: 'icon', + display: 'base', + } as TopNavControlIconData, + ]} + setMountPoint={application.setAppRightControls} + /> + )} + { type: 'button', description: 'Delete workspace', 'data-test-subj': 'workspace-list-delete-icon', + available: () => isDashboardAdmin, render: (item: WorkspaceAttribute) => { return ( Date: Thu, 5 Sep 2024 17:39:57 +0800 Subject: [PATCH 03/12] [Workspace]Hide saved object import button when user is outside workspace (#7989) * Hide saved object import button when user is outside workpace Signed-off-by: yubonluo * Changeset file for PR #7989 created/updated --------- Signed-off-by: yubonluo Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/7989.yml | 2 + .../saved_objects_table.test.tsx.snap | 1 + .../__snapshots__/header.test.tsx.snap | 44 +++++++++++++++ .../objects_table/components/header.test.tsx | 45 +++++++++++++++ .../objects_table/components/header.tsx | 55 +++++++++++-------- .../objects_table/saved_objects_table.tsx | 1 + 6 files changed, 126 insertions(+), 22 deletions(-) create mode 100644 changelogs/fragments/7989.yml diff --git a/changelogs/fragments/7989.yml b/changelogs/fragments/7989.yml new file mode 100644 index 000000000000..401774e8b711 --- /dev/null +++ b/changelogs/fragments/7989.yml @@ -0,0 +1,2 @@ +refactor: +- Hide saved object import button when user is outside workspace ([#7989](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7989)) \ No newline at end of file diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/__snapshots__/saved_objects_table.test.tsx.snap b/src/plugins/saved_objects_management/public/management_section/objects_table/__snapshots__/saved_objects_table.test.tsx.snap index 799535aec1c7..c3ba3dfea905 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/__snapshots__/saved_objects_table.test.tsx.snap +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/__snapshots__/saved_objects_table.test.tsx.snap @@ -1042,6 +1042,7 @@ exports[`SavedObjectsTable should render normally 1`] = ` onExportAll={[Function]} onImport={[Function]} onRefresh={[Function]} + showImportButton={true} /> + + + + + + +`; + exports[`Header should render normally 1`] = ` null, TopNavMenu: () => null }, applications: applicationServiceMock.createStartContract(), + showImportButton: true, }; describe('Header', () => { @@ -92,4 +93,48 @@ describe('Header - workspace enabled', () => { expect(component.find('EuiButtonEmpty[data-test-subj="duplicateObjects"]').exists()).toBe(true); }); + + it('should render `Import` button inside a workspace', () => { + const props = { + ...defaultProps, + showImportButton: true, + }; + + const component = shallow(
); + + expect(component.find('EuiButtonEmpty[data-test-subj="importObjects"]').exists()).toBe(true); + + const newUxProps = { + ...defaultProps, + showImportButton: true, + useUpdatedUX: true, + }; + + const newUxComponent = shallow(
); + + expect(newUxComponent).toMatchSnapshot(); + }); + + it('should not render `Import` button outside a workspace', () => { + const props = { + ...defaultProps, + showImportButton: false, + }; + + const component = shallow(
); + + expect(component.find('EuiButtonEmpty[data-test-subj="importObjects"]').exists()).toBe(false); + + const newUxProps = { + ...defaultProps, + showImportButton: true, + useUpdatedUX: false, + }; + + const newUxComponent = shallow(
); + + expect(newUxComponent.find('EuiButtonEmpty[data-test-subj="importObjects"]').exists()).toBe( + true + ); + }); }); diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/components/header.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/components/header.tsx index 911db61be704..9ac49f007057 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/components/header.tsx +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/components/header.tsx @@ -56,6 +56,7 @@ export const Header = ({ navigationUI: { HeaderControl }, applications, currentWorkspaceName, + showImportButton, }: { onExportAll: () => void; onImport: () => void; @@ -67,6 +68,7 @@ export const Header = ({ navigationUI: NavigationPublicPluginStart['ui']; applications: ApplicationStart; currentWorkspaceName: string; + showImportButton: boolean; }) => { const title = useUpdatedUX ? null : ( @@ -143,15 +145,22 @@ export const Header = ({ defaultMessage: 'Export all objects', }), } as TopNavControlButtonData, - { - testId: 'importObjects', - run: onImport, - controlType: 'button', - iconType: 'importAction', - label: i18n.translate('savedObjectsManagement.objectsTable.header.importButtonLabel', { - defaultMessage: 'Import', - }), - } as TopNavControlButtonData, + ...(showImportButton + ? [ + { + testId: 'importObjects', + run: onImport, + controlType: 'button', + iconType: 'importAction', + label: i18n.translate( + 'savedObjectsManagement.objectsTable.header.importButtonLabel', + { + defaultMessage: 'Import', + } + ), + } as TopNavControlButtonData, + ] + : []), ]} setMountPoint={applications.setAppRightControls} /> @@ -187,19 +196,21 @@ export const Header = ({ /> - - - - - + {showImportButton && ( + + + + + + )} {!useUpdatedUX && } From 63d0d328dd8c6582c4dd6838663deebc44f14ba3 Mon Sep 17 00:00:00 2001 From: Hailong Cui Date: Thu, 5 Sep 2024 17:53:37 +0800 Subject: [PATCH 04/12] [Workspace] Hide first home breadcrumbs within workspace (#7992) * hide first home breadcrumbs within workspace Signed-off-by: Hailong Cui * Changeset file for PR #7992 created/updated * don't relay on text to drop home Signed-off-by: Hailong Cui * don't relay on text to drop home Signed-off-by: Hailong Cui --------- Signed-off-by: Hailong Cui Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/7992.yml | 2 + .../header/__snapshots__/header.test.tsx.snap | 204 +++++++++++++++++- src/core/public/chrome/ui/header/header.tsx | 8 +- .../ui/header/header_breadcrumbs.test.tsx | 40 ++++ .../chrome/ui/header/header_breadcrumbs.tsx | 6 + src/plugins/workspace/public/utils.ts | 3 +- 6 files changed, 254 insertions(+), 9 deletions(-) create mode 100644 changelogs/fragments/7992.yml diff --git a/changelogs/fragments/7992.yml b/changelogs/fragments/7992.yml new file mode 100644 index 000000000000..b73b8b4e07ef --- /dev/null +++ b/changelogs/fragments/7992.yml @@ -0,0 +1,2 @@ +feat: +- [Workspace] Hide home breadcrumbs when in a workspace ([#7992](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7992)) \ No newline at end of file diff --git a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap index d9b543a32627..96847daa9271 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap @@ -468,7 +468,45 @@ exports[`Header handles visibility and lock changes 1`] = ` "closed": false, "hasError": false, "isStopped": false, - "observers": Array [], + "observers": Array [ + Subscriber { + "_parentOrParents": null, + "_subscriptions": Array [ + SubjectSubscription { + "_parentOrParents": [Circular], + "_subscriptions": null, + "closed": false, + "subject": [Circular], + "subscriber": [Circular], + }, + ], + "closed": false, + "destination": SafeSubscriber { + "_complete": undefined, + "_context": [Circular], + "_error": undefined, + "_next": [Function], + "_parentOrParents": null, + "_parentSubscriber": [Circular], + "_subscriptions": null, + "closed": false, + "destination": Object { + "closed": true, + "complete": [Function], + "error": [Function], + "next": [Function], + }, + "isStopped": false, + "syncErrorThrowable": false, + "syncErrorThrown": false, + "syncErrorValue": null, + }, + "isStopped": false, + "syncErrorThrowable": true, + "syncErrorThrown": false, + "syncErrorValue": null, + }, + ], "thrownError": null, } } @@ -7540,7 +7578,45 @@ exports[`Header renders application header without title and breadcrumbs 1`] = ` "closed": false, "hasError": false, "isStopped": false, - "observers": Array [], + "observers": Array [ + Subscriber { + "_parentOrParents": null, + "_subscriptions": Array [ + SubjectSubscription { + "_parentOrParents": [Circular], + "_subscriptions": null, + "closed": false, + "subject": [Circular], + "subscriber": [Circular], + }, + ], + "closed": false, + "destination": SafeSubscriber { + "_complete": undefined, + "_context": [Circular], + "_error": undefined, + "_next": [Function], + "_parentOrParents": null, + "_parentSubscriber": [Circular], + "_subscriptions": null, + "closed": false, + "destination": Object { + "closed": true, + "complete": [Function], + "error": [Function], + "next": [Function], + }, + "isStopped": false, + "syncErrorThrowable": false, + "syncErrorThrown": false, + "syncErrorValue": null, + }, + "isStopped": false, + "syncErrorThrowable": true, + "syncErrorThrown": false, + "syncErrorValue": null, + }, + ], "thrownError": null, } } @@ -8843,6 +8919,7 @@ exports[`Header renders application header without title and breadcrumbs 1`] = ` "thrownError": null, } } + dropHomeFromBreadcrumb={false} renderFullLength={true} useUpdatedHeader={true} /> @@ -10100,7 +10177,45 @@ exports[`Header renders condensed header 1`] = ` "closed": false, "hasError": false, "isStopped": false, - "observers": Array [], + "observers": Array [ + Subscriber { + "_parentOrParents": null, + "_subscriptions": Array [ + SubjectSubscription { + "_parentOrParents": [Circular], + "_subscriptions": null, + "closed": false, + "subject": [Circular], + "subscriber": [Circular], + }, + ], + "closed": false, + "destination": SafeSubscriber { + "_complete": undefined, + "_context": [Circular], + "_error": undefined, + "_next": [Function], + "_parentOrParents": null, + "_parentSubscriber": [Circular], + "_subscriptions": null, + "closed": false, + "destination": Object { + "closed": true, + "complete": [Function], + "error": [Function], + "next": [Function], + }, + "isStopped": false, + "syncErrorThrowable": false, + "syncErrorThrown": false, + "syncErrorValue": null, + }, + "isStopped": false, + "syncErrorThrowable": true, + "syncErrorThrown": false, + "syncErrorValue": null, + }, + ], "thrownError": null, } } @@ -15743,7 +15858,45 @@ exports[`Header renders page header with application title 1`] = ` "closed": false, "hasError": false, "isStopped": false, - "observers": Array [], + "observers": Array [ + Subscriber { + "_parentOrParents": null, + "_subscriptions": Array [ + SubjectSubscription { + "_parentOrParents": [Circular], + "_subscriptions": null, + "closed": false, + "subject": [Circular], + "subscriber": [Circular], + }, + ], + "closed": false, + "destination": SafeSubscriber { + "_complete": undefined, + "_context": [Circular], + "_error": undefined, + "_next": [Function], + "_parentOrParents": null, + "_parentSubscriber": [Circular], + "_subscriptions": null, + "closed": false, + "destination": Object { + "closed": true, + "complete": [Function], + "error": [Function], + "next": [Function], + }, + "isStopped": false, + "syncErrorThrowable": false, + "syncErrorThrown": false, + "syncErrorValue": null, + }, + "isStopped": false, + "syncErrorThrowable": true, + "syncErrorThrown": false, + "syncErrorValue": null, + }, + ], "thrownError": null, } } @@ -17272,6 +17425,7 @@ exports[`Header renders page header with application title 1`] = ` "thrownError": null, } } + dropHomeFromBreadcrumb={false} renderFullLength={true} useUpdatedHeader={true} /> @@ -17617,6 +17771,8 @@ exports[`Header renders page header with application title 1`] = ` "thrownError": null, } } + dropHomeFromBreadcrumb={false} + renderFullLength={false} useUpdatedHeader={true} > { return getOsdSidecarPaddingStyle(sidecarConfig); @@ -203,13 +204,14 @@ export function Header({ /> ); - const renderBreadcrumbs = (renderFullLength?: boolean) => ( + const renderBreadcrumbs = (renderFullLength?: boolean, dropHomeFromBreadcrumb?: boolean) => ( ); @@ -351,7 +353,7 @@ export function Header({ recentlyAccessed$={observables.recentlyAccessed$} workspaceList$={observables.workspaceList$} navigateToUrl={application.navigateToUrl} - renderBreadcrumbs={renderBreadcrumbs(true)} + renderBreadcrumbs={renderBreadcrumbs(true, false)} buttonSize={useApplicationHeader ? 's' : 'xs'} /> @@ -399,7 +401,7 @@ export function Header({ {renderRecentItems()} - {renderBreadcrumbs()} + {renderBreadcrumbs(false, !!currentWorkspace)} {/* Secondary header */} diff --git a/src/core/public/chrome/ui/header/header_breadcrumbs.test.tsx b/src/core/public/chrome/ui/header/header_breadcrumbs.test.tsx index f01e751e39f0..2e3c7f753e4c 100644 --- a/src/core/public/chrome/ui/header/header_breadcrumbs.test.tsx +++ b/src/core/public/chrome/ui/header/header_breadcrumbs.test.tsx @@ -107,4 +107,44 @@ describe('HeaderBreadcrumbs', () => { wrapper.update(); expect(wrapper.find('.euiBreadcrumbWrapper')).toHaveLength(2); }); + + it('remove heading home when workspace is enabled', () => { + const breadcrumbs$ = new BehaviorSubject([{ text: 'First' }]); + const breadcrumbsEnricher$ = new BehaviorSubject((crumbs: ChromeBreadcrumb[]) => [ + { text: 'Home', home: true }, + { text: 'Analytics' }, + ...crumbs, + ]); + const wrapper = mount( + + ); + let breadcrumbs = wrapper.find('.euiBreadcrumbWrapper'); + expect(breadcrumbs).toHaveLength(2); + expect(breadcrumbs.at(0).text()).toBe('Analytics'); + expect(breadcrumbs.at(1).text()).toBe('First'); + + // if no home property, we don't drop by text + act(() => { + breadcrumbsEnricher$.next((items) => items); + breadcrumbs$.next([{ text: 'Home' }, { text: 'Second' }]); + }); + wrapper.update(); + breadcrumbs = wrapper.find('.euiBreadcrumbWrapper'); + expect(breadcrumbs).toHaveLength(2); + expect(breadcrumbs.at(0).text()).toBe('Home'); + expect(breadcrumbs.at(1).text()).toBe('Second'); + + act(() => { + breadcrumbsEnricher$.next((items) => []); + breadcrumbs$.next([{ text: 'Home' }, { text: 'Second' }]); + }); + wrapper.update(); + breadcrumbs = wrapper.find('.euiBreadcrumbWrapper'); + expect(breadcrumbs).toHaveLength(0); + }); }); diff --git a/src/core/public/chrome/ui/header/header_breadcrumbs.tsx b/src/core/public/chrome/ui/header/header_breadcrumbs.tsx index 52c9aa2628e9..35ef40f0209f 100644 --- a/src/core/public/chrome/ui/header/header_breadcrumbs.tsx +++ b/src/core/public/chrome/ui/header/header_breadcrumbs.tsx @@ -41,6 +41,7 @@ interface Props { breadcrumbsEnricher$: Observable; useUpdatedHeader?: boolean; renderFullLength?: boolean; + dropHomeFromBreadcrumb?: boolean; } export function HeaderBreadcrumbs({ @@ -49,6 +50,7 @@ export function HeaderBreadcrumbs({ breadcrumbsEnricher$, useUpdatedHeader, renderFullLength, + dropHomeFromBreadcrumb, }: Props) { const appTitle = useObservable(appTitle$, 'OpenSearch Dashboards'); const breadcrumbs = useObservable(breadcrumbs$, []); @@ -73,6 +75,10 @@ export function HeaderBreadcrumbs({ crumbs = breadcrumbEnricher(crumbs); } + if (dropHomeFromBreadcrumb && crumbs.length && Object.hasOwn(crumbs[0], 'home')) { + crumbs = crumbs.slice(1); + } + crumbs = crumbs.map((breadcrumb, i) => ({ ...breadcrumb, 'data-test-subj': classNames( diff --git a/src/plugins/workspace/public/utils.ts b/src/plugins/workspace/public/utils.ts index 2620b4703670..bbc3d0dbd6a1 100644 --- a/src/plugins/workspace/public/utils.ts +++ b/src/plugins/workspace/public/utils.ts @@ -385,8 +385,9 @@ export function prependWorkspaceToBreadcrumbs( return; } - const homeBreadcrumb: ChromeBreadcrumb = { + const homeBreadcrumb: ChromeBreadcrumb & { home: boolean } = { text: 'Home', + home: true, onClick: () => { core.application.navigateToApp('home'); }, From a4470a56de5e92bb31f8b0b368adedb9f62b96c6 Mon Sep 17 00:00:00 2001 From: Qxisylolo Date: Thu, 5 Sep 2024 17:55:19 +0800 Subject: [PATCH 05/12] [Workspace] add use case cards when workspace is disabled, refine the style of recent_work in the home page. (#7967) * 04-fix-conflication Signed-off-by: Qxisylolo * 02-add-icon Signed-off-by: Qxisylolo * 02-add cards and resolve recent cards Signed-off-by: Qxisylolo * 04-fix-bug Signed-off-by: Qxisylolo * 04-fix-conflication Signed-off-by: Qxisylolo * 05-add-tests Signed-off-by: Qxisylolo * 05-add-tests-1 Signed-off-by: Qxisylolo * 06-enable-navigate-to-overview-page Signed-off-by: Qxisylolo --------- Signed-off-by: Qxisylolo --- src/core/utils/default_nav_groups.ts | 9 +- .../components/use_case_card.test.ts | 78 ++++++++++ .../application/components/use_case_card.ts | 55 +++++++ .../public/application/home_render.test.tsx | 142 ++++++++++++++++++ .../home/public/application/home_render.tsx | 47 +++++- .../public/management_section/recent_work.tsx | 79 +++++----- src/plugins/workspace/common/constants.ts | 17 ++- .../use_case_overview/setup_overview.test.tsx | 2 +- 8 files changed, 370 insertions(+), 59 deletions(-) create mode 100644 src/plugins/home/public/application/components/use_case_card.test.ts create mode 100644 src/plugins/home/public/application/components/use_case_card.ts create mode 100644 src/plugins/home/public/application/home_render.test.tsx diff --git a/src/core/utils/default_nav_groups.ts b/src/core/utils/default_nav_groups.ts index 7278734ae826..bbd38c81ffc5 100644 --- a/src/core/utils/default_nav_groups.ts +++ b/src/core/utils/default_nav_groups.ts @@ -52,8 +52,7 @@ const defaultNavGroups = { defaultMessage: 'Observability', }), description: i18n.translate('core.ui.group.observability.description', { - defaultMessage: - 'Gain visibility into system health, performance, and reliability through monitoring and analysis of logs, metrics, and traces.', + defaultMessage: 'Gain visibility into your application and infrastructure', }), order: 4000, icon: 'wsObservability', @@ -64,8 +63,7 @@ const defaultNavGroups = { defaultMessage: 'Security Analytics', }), description: i18n.translate('core.ui.group.security.analytics.description', { - defaultMessage: - 'Detect and investigate potential security threats and vulnerabilities across your systems and data.', + defaultMessage: 'Enhance your security posture with advanced analytics', }), order: 5000, icon: 'wsSecurityAnalytics', @@ -88,8 +86,7 @@ const defaultNavGroups = { defaultMessage: 'Search', }), description: i18n.translate('core.ui.group.search.description', { - defaultMessage: - "Quickly find and explore relevant information across your organization's data sources.", + defaultMessage: 'Discover and query your data with ease', }), order: 6000, icon: 'wsSearch', diff --git a/src/plugins/home/public/application/components/use_case_card.test.ts b/src/plugins/home/public/application/components/use_case_card.test.ts new file mode 100644 index 000000000000..e26e6b1fb0eb --- /dev/null +++ b/src/plugins/home/public/application/components/use_case_card.test.ts @@ -0,0 +1,78 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { EuiIcon } from '@elastic/eui'; +import { coreMock } from '../../../../../core/public/mocks'; +import { registerUseCaseCard } from './use_case_card'; +import { contentManagementPluginMocks } from '../../../../content_management/public'; + +describe('registerUseCaseCard', () => { + const registerContentProviderFn = jest.fn(); + const contentManagementStartMock = { + ...contentManagementPluginMocks.createStartContract(), + registerContentProvider: registerContentProviderFn, + }; + + const core = coreMock.createStart(); + + it('should register useCase card correctly', () => { + registerUseCaseCard(contentManagementStartMock, core, { + id: 'testId', + order: 1, + target: 'osd_homepage/get_started', + icon: 'wsObservability', + title: 'observability', + description: 'Gain visibility into your application and infrastructure', + navigateAppId: 'observability_overview', + }); + + expect(contentManagementStartMock.registerContentProvider).toHaveBeenCalledTimes(1); + + const registerCall = contentManagementStartMock.registerContentProvider.mock.calls[0][0]; + + expect(registerCall.getTargetArea()).toEqual('osd_homepage/get_started'); + + expect(registerCall.getContent()).toEqual({ + id: 'testId', + kind: 'card', + order: 1, + description: 'Gain visibility into your application and infrastructure', + title: 'observability', + cardProps: { + layout: 'horizontal', + }, + onClick: expect.any(Function), + getIcon: expect.any(Function), + }); + + const icon = registerCall.getContent().getIcon(); + expect(icon.type).toBe(EuiIcon); + expect(icon.props).toEqual({ + size: 'l', + color: 'subdued', + type: 'wsObservability', + }); + }); + + it('should be able to navigate to the expected overview page when click the card', () => { + const navigateToAppMock = jest.fn(); + core.application.navigateToApp = navigateToAppMock; + + registerUseCaseCard(contentManagementStartMock, core, { + id: 'testId', + order: 1, + target: 'osd_homepage/get_started', + icon: 'wsObservability', + title: 'observability', + description: 'Gain visibility into your application and infrastructure', + navigateAppId: 'observability_overview', + }); + + const registerCall = contentManagementStartMock.registerContentProvider.mock.calls[0][0]; + const card = registerCall.getContent(); + card.onClick(); + expect(navigateToAppMock).toHaveBeenCalledWith('observability_overview'); + }); +}); diff --git a/src/plugins/home/public/application/components/use_case_card.ts b/src/plugins/home/public/application/components/use_case_card.ts new file mode 100644 index 000000000000..b71f381358f4 --- /dev/null +++ b/src/plugins/home/public/application/components/use_case_card.ts @@ -0,0 +1,55 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { CoreStart } from 'opensearch-dashboards/public'; +import { EuiIcon } from '@elastic/eui'; +import { ContentManagementPluginStart } from '../../../../content_management/public'; + +export const registerUseCaseCard = ( + contentManagement: ContentManagementPluginStart, + core: CoreStart, + { + target, + order, + id, + title, + description, + icon, + navigateAppId, + }: { + target: string; + order: number; + id: string; + title: string; + description: string; + icon: string; + navigateAppId: string; + } +) => { + contentManagement.registerContentProvider({ + id: `home_get_started_${id}`, + getTargetArea: () => target, + getContent: () => ({ + id, + kind: 'card', + order, + description, + title, + cardProps: { + layout: 'horizontal', + }, + onClick: () => { + core.application.navigateToApp(navigateAppId); + }, + getIcon: () => + React.createElement(EuiIcon, { + size: 'l', + color: 'subdued', + type: icon, + }), + }), + }); +}; diff --git a/src/plugins/home/public/application/home_render.test.tsx b/src/plugins/home/public/application/home_render.test.tsx new file mode 100644 index 000000000000..34a5d690b44b --- /dev/null +++ b/src/plugins/home/public/application/home_render.test.tsx @@ -0,0 +1,142 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { DEFAULT_NAV_GROUPS } from '../../../../core/public'; +import { + HOME_CONTENT_AREAS, + SEARCH_OVERVIEW_PAGE_ID, + OBSERVABILITY_OVERVIEW_PAGE_ID, + SECURITY_ANALYTICS_OVERVIEW_PAGE_ID, +} from '../../../../plugins/content_management/public'; +import { contentManagementPluginMocks } from '../../../../plugins/content_management/public/mocks'; +import { registerUseCaseCard } from './components/use_case_card'; +import { initHome } from './home_render'; + +import { + WHATS_NEW_CONFIG, + LEARN_OPENSEARCH_CONFIG, + registerHomeListCard, +} from './components/home_list_card'; + +jest.mock('./components/use_case_card', () => ({ + registerUseCaseCard: jest.fn(), +})); + +jest.mock('./components/home_list_card', () => ({ + registerHomeListCard: jest.fn(), +})); + +describe('initHome', () => { + const registerContentProviderFn = jest.fn(); + const contentManagementStartMock = { + ...contentManagementPluginMocks.createStartContract(), + registerContentProvider: registerContentProviderFn, + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should register use case cards when workspace is enabled', () => { + const coreMock = { + createStart: jest.fn(() => ({ + application: { + capabilities: { + workspaces: { + enabled: false, + }, + }, + navigateToApp: jest.fn(), + }, + })), + }; + const core = coreMock.createStart(); + + initHome(contentManagementStartMock, core); + + expect(registerUseCaseCard).toHaveBeenCalledTimes(3); + + expect(registerUseCaseCard).toHaveBeenCalledWith(contentManagementStartMock, core, { + id: DEFAULT_NAV_GROUPS.observability.id, + order: 1, + description: DEFAULT_NAV_GROUPS.observability.description, + title: DEFAULT_NAV_GROUPS.observability.title, + target: HOME_CONTENT_AREAS.GET_STARTED, + icon: DEFAULT_NAV_GROUPS.observability.icon ?? '', + navigateAppId: OBSERVABILITY_OVERVIEW_PAGE_ID, + }); + + expect(registerUseCaseCard).toHaveBeenCalledWith(contentManagementStartMock, core, { + id: DEFAULT_NAV_GROUPS.search.id, + order: 2, + description: DEFAULT_NAV_GROUPS.search.description, + title: DEFAULT_NAV_GROUPS.search.title, + target: HOME_CONTENT_AREAS.GET_STARTED, + icon: DEFAULT_NAV_GROUPS.search.icon ?? '', + navigateAppId: SEARCH_OVERVIEW_PAGE_ID, + }); + + expect(registerUseCaseCard).toHaveBeenCalledWith(contentManagementStartMock, core, { + id: DEFAULT_NAV_GROUPS['security-analytics'].id, + order: 3, + description: DEFAULT_NAV_GROUPS['security-analytics'].description, + title: DEFAULT_NAV_GROUPS['security-analytics'].title, + target: HOME_CONTENT_AREAS.GET_STARTED, + icon: DEFAULT_NAV_GROUPS['security-analytics'].icon ?? '', + navigateAppId: SECURITY_ANALYTICS_OVERVIEW_PAGE_ID, + }); + }); + + it('should not register use case cards when workspace is disabled', () => { + const coreMock = { + createStart: jest.fn(() => ({ + application: { + capabilities: { + workspaces: { + enabled: true, + }, + }, + }, + })), + }; + const core = coreMock.createStart(); + initHome(contentManagementStartMock, core); + expect(registerUseCaseCard).not.toHaveBeenCalled(); + }); + + it('should register home list cards correctly', () => { + const coreMock = { + createStart: jest.fn(() => ({ + application: { + capabilities: { + workspaces: { + enabled: false, + }, + }, + }, + })), + }; + const core = coreMock.createStart(); + initHome(contentManagementStartMock, core); + + expect(registerHomeListCard).toHaveBeenCalledTimes(2); + + expect(registerHomeListCard).toHaveBeenCalledWith(contentManagementStartMock, { + id: 'whats_new', + order: 10, + config: WHATS_NEW_CONFIG, + target: HOME_CONTENT_AREAS.SERVICE_CARDS, + width: 16, + }); + + expect(registerHomeListCard).toHaveBeenCalledWith(contentManagementStartMock, { + id: 'learn_opensearch_new', + order: 11, + config: LEARN_OPENSEARCH_CONFIG, + target: HOME_CONTENT_AREAS.SERVICE_CARDS, + width: 16, + }); + }); +}); diff --git a/src/plugins/home/public/application/home_render.tsx b/src/plugins/home/public/application/home_render.tsx index ef73d7c44d99..f22f3560205c 100644 --- a/src/plugins/home/public/application/home_render.tsx +++ b/src/plugins/home/public/application/home_render.tsx @@ -5,12 +5,16 @@ import React from 'react'; import { CoreStart } from 'opensearch-dashboards/public'; +import { DEFAULT_NAV_GROUPS } from '../../../../core/public'; import { ContentManagementPluginSetup, ContentManagementPluginStart, HOME_PAGE_ID, SECTIONS, HOME_CONTENT_AREAS, + SEARCH_OVERVIEW_PAGE_ID, + OBSERVABILITY_OVERVIEW_PAGE_ID, + SECURITY_ANALYTICS_OVERVIEW_PAGE_ID, } from '../../../../plugins/content_management/public'; import { WHATS_NEW_CONFIG, @@ -18,16 +22,13 @@ import { registerHomeListCard, } from './components/home_list_card'; +import { registerUseCaseCard } from './components/use_case_card'; + export const setupHome = (contentManagement: ContentManagementPluginSetup) => { contentManagement.registerPage({ id: HOME_PAGE_ID, title: 'Home', sections: [ - { - id: SECTIONS.SERVICE_CARDS, - order: 3000, - kind: 'dashboard', - }, { id: SECTIONS.RECENTLY_VIEWED, order: 2000, @@ -47,10 +48,15 @@ export const setupHome = (contentManagement: ContentManagementPluginSetup) => { ); }, }, + { + id: SECTIONS.SERVICE_CARDS, + order: 3000, + kind: 'dashboard', + }, { id: SECTIONS.GET_STARTED, order: 1000, - title: 'Get started with OpenSearch’s powerful features', + title: "Get started with OpenSearch's powerful features", kind: 'card', }, ], @@ -58,9 +64,34 @@ export const setupHome = (contentManagement: ContentManagementPluginSetup) => { }; export const initHome = (contentManagement: ContentManagementPluginStart, core: CoreStart) => { + const workspaceEnabled = core.application.capabilities.workspaces.enabled; + + if (!workspaceEnabled) { + const useCases = [ + { ...DEFAULT_NAV_GROUPS.observability, navigateAppId: OBSERVABILITY_OVERVIEW_PAGE_ID }, + { ...DEFAULT_NAV_GROUPS.search, navigateAppId: SEARCH_OVERVIEW_PAGE_ID }, + { + ...DEFAULT_NAV_GROUPS['security-analytics'], + navigateAppId: SECURITY_ANALYTICS_OVERVIEW_PAGE_ID, + }, + ]; + + useCases.forEach((useCase, index) => { + registerUseCaseCard(contentManagement, core, { + id: useCase.id, + order: index + 1, + description: useCase.description, + title: useCase.title, + target: HOME_CONTENT_AREAS.GET_STARTED, + icon: useCase.icon ?? '', + navigateAppId: useCase.navigateAppId, + }); + }); + } + registerHomeListCard(contentManagement, { id: 'whats_new', - order: 3, + order: 10, config: WHATS_NEW_CONFIG, target: HOME_CONTENT_AREAS.SERVICE_CARDS, width: 16, @@ -68,7 +99,7 @@ export const initHome = (contentManagement: ContentManagementPluginStart, core: registerHomeListCard(contentManagement, { id: 'learn_opensearch_new', - order: 4, + order: 11, config: LEARN_OPENSEARCH_CONFIG, target: HOME_CONTENT_AREAS.SERVICE_CARDS, width: 16, diff --git a/src/plugins/saved_objects_management/public/management_section/recent_work.tsx b/src/plugins/saved_objects_management/public/management_section/recent_work.tsx index eb04652d6343..88cdfc142af4 100644 --- a/src/plugins/saved_objects_management/public/management_section/recent_work.tsx +++ b/src/plugins/saved_objects_management/public/management_section/recent_work.tsx @@ -257,38 +257,45 @@ export const RecentWork = (props: { core: CoreStart; workspaceEnabled?: boolean textAlign="left" href={recentNavLink.href} footer={ - <> - - - - {selectedSort === recentlyViewed - ? i18n.translate( - 'savedObjectsManagement.recentWorkSection.viewedAt', - { - defaultMessage: 'Viewed', - } - ) - : i18n.translate( - 'savedObjectsManagement.recentWorkSection.updatedAt', - { - defaultMessage: 'Updated', - } - )} - :{' '} - - - - - + + + + + {selectedSort === recentlyViewed - ? moment(recentAccessItem?.lastAccessedTime).fromNow() - : moment(recentAccessItem?.updatedAt).fromNow()} - - - - - {workspaceEnabled && ( - <> + ? i18n.translate( + 'savedObjectsManagement.recentWorkSection.viewedAt', + { + defaultMessage: 'Viewed', + } + ) + : i18n.translate( + 'savedObjectsManagement.recentWorkSection.updatedAt', + { + defaultMessage: 'Updated', + } + )} + :{' '} + + + + + + {selectedSort === recentlyViewed + ? moment(recentAccessItem?.lastAccessedTime).fromNow() + : moment(recentAccessItem?.updatedAt).fromNow()} + + + + + + {workspaceEnabled && ( + + {i18n.translate( @@ -300,15 +307,15 @@ export const RecentWork = (props: { core: CoreStart; workspaceEnabled?: boolean : - + {recentAccessItem.workspaceName || 'N/A'} - - )} - - + + + )} + } onClick={recentNavLink.onClick} /> diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index f66f7ea71752..22c746c0dbed 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -45,6 +45,7 @@ export const PRIORITY_FOR_PERMISSION_CONTROL_WRAPPER = 0; * store a static map in workspace. * */ + export const WORKSPACE_USE_CASES = Object.freeze({ observability: { id: 'observability', @@ -52,9 +53,9 @@ export const WORKSPACE_USE_CASES = Object.freeze({ defaultMessage: 'Observability', }), description: i18n.translate('workspace.usecase.observability.description', { - defaultMessage: - 'Gain visibility into system health, performance, and reliability through monitoring and analysis of logs, metrics, and traces.', + defaultMessage: 'Gain visibility into your application and infrastructure', }), + icon: 'wsObservability', features: [ 'discover', 'dashboards', @@ -78,9 +79,9 @@ export const WORKSPACE_USE_CASES = Object.freeze({ defaultMessage: 'Security Analytics', }), description: i18n.translate('workspace.usecase.analytics.description', { - defaultMessage: - 'Detect and investigate potential security threats and vulnerabilities across your systems and data.', + defaultMessage: 'Enhance your security posture with advanced analytics', }), + icon: 'wsSecurityAnalytics', features: [ 'discover', 'dashboards', @@ -102,9 +103,9 @@ export const WORKSPACE_USE_CASES = Object.freeze({ defaultMessage: 'Essentials', }), description: i18n.translate('workspace.usecase.essentials.description', { - defaultMessage: - 'Analyze data to derive insights, identify patterns and trends, and make data-driven decisions.', + defaultMessage: 'Get start with just the basics', }), + icon: 'wsEssentials', features: [ 'discover', 'dashboards', @@ -125,9 +126,9 @@ export const WORKSPACE_USE_CASES = Object.freeze({ defaultMessage: 'Search', }), description: i18n.translate('workspace.usecase.search.description', { - defaultMessage: - "Quickly find and explore relevant information across your organization's data sources.", + defaultMessage: 'Discover and query your data with ease', }), + icon: 'wsSearch', features: [ 'discover', 'dashboards', diff --git a/src/plugins/workspace/public/components/use_case_overview/setup_overview.test.tsx b/src/plugins/workspace/public/components/use_case_overview/setup_overview.test.tsx index cdcebca33e19..e1f47c9756d1 100644 --- a/src/plugins/workspace/public/components/use_case_overview/setup_overview.test.tsx +++ b/src/plugins/workspace/public/components/use_case_overview/setup_overview.test.tsx @@ -137,7 +137,7 @@ describe('Setup use case overview', () => { "cardProps": Object { "layout": "horizontal", }, - "description": "Gain visibility into system health, performance, and reliability through monitoring and analysis of logs, metrics, and traces.", + "description": "Gain visibility into your application and infrastructure", "getIcon": [Function], "id": "observability", "kind": "card", From a05c4a2e5b44a0811d937a5069b53337882da1d0 Mon Sep 17 00:00:00 2001 From: Qxisylolo Date: Thu, 5 Sep 2024 17:57:37 +0800 Subject: [PATCH 06/12] [Workspace] Content menu picker in side bar and enable searching (#7881) * 01-feat/refractor-content-menu-picker-in-side-bar Signed-off-by: Qxisylolo * 02-enable-associated-icon Signed-off-by: Qxisylolo * 03-update-side-bar Signed-off-by: Qxisylolo * 04-fix-bugs Signed-off-by: Qxisylolo * 04-fix-bugs, add test Signed-off-by: Qxisylolo * 05-delete search Signed-off-by: Qxisylolo * 06-delete search Signed-off-by: Qxisylolo * 06-fix-bug Signed-off-by: Qxisylolo * 07-fix-conflication-1 Signed-off-by: Qxisylolo * 07-fix-conflication-adjust-icon-size Signed-off-by: Qxisylolo * Changeset file for PR #7881 created/updated --------- Signed-off-by: Qxisylolo Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/7881.yml | 2 + .../workspace_menu/workspace_menu.test.tsx | 75 ++++++---- .../workspace_menu/workspace_menu.tsx | 137 ++++++++---------- .../workspace_picker_content.tsx | 133 ++++++++++++++--- 4 files changed, 220 insertions(+), 127 deletions(-) create mode 100644 changelogs/fragments/7881.yml diff --git a/changelogs/fragments/7881.yml b/changelogs/fragments/7881.yml new file mode 100644 index 000000000000..49da3e4fa2e5 --- /dev/null +++ b/changelogs/fragments/7881.yml @@ -0,0 +1,2 @@ +feat: +- Refactor content menu picker in side bar and enable searching ([#7881](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7881)) \ No newline at end of file diff --git a/src/plugins/workspace/public/components/workspace_menu/workspace_menu.test.tsx b/src/plugins/workspace/public/components/workspace_menu/workspace_menu.test.tsx index 39acffd0c42d..a0afcf3eb8bb 100644 --- a/src/plugins/workspace/public/components/workspace_menu/workspace_menu.test.tsx +++ b/src/plugins/workspace/public/components/workspace_menu/workspace_menu.test.tsx @@ -12,7 +12,6 @@ import { CoreStart, DEFAULT_NAV_GROUPS } from '../../../../../core/public'; import { BehaviorSubject } from 'rxjs'; import { IntlProvider } from 'react-intl'; import { recentWorkspaceManager } from '../../recent_workspace_manager'; -import * as workspaceUtils from '../utils/workspace'; describe('', () => { let coreStartMock: CoreStart; @@ -91,7 +90,52 @@ describe('', () => { expect(screen.getByTestId('workspace-menu-item-recent-workspace-2')).toBeInTheDocument(); }); - it('should display current workspace name and use case name', () => { + it('should be able to display empty state when the workspace list is empty', () => { + coreStartMock.workspaces.workspaceList$.next([]); + render(); + const selectButton = screen.getByTestId('workspace-select-button'); + fireEvent.click(selectButton); + expect(screen.getByText(/no workspace available/i)).toBeInTheDocument(); + }); + + it('should be able to perform search and filter and the results will be shown in both all and recent section', () => { + coreStartMock.workspaces.workspaceList$.next([ + { id: 'workspace-1', name: 'workspace 1', features: [] }, + { id: 'test-2', name: 'test 2', features: [] }, + ]); + jest + .spyOn(recentWorkspaceManager, 'getRecentWorkspaces') + .mockReturnValue([{ id: 'workspace-1', timestamp: 1234567890 }]); + render(); + + const selectButton = screen.getByTestId('workspace-select-button'); + fireEvent.click(selectButton); + + const searchInput = screen.getByRole('searchbox'); + fireEvent.change(searchInput, { target: { value: 'works' } }); + expect(screen.getByTestId('workspace-menu-item-recent-workspace-1')).toBeInTheDocument(); + expect(screen.getByTestId('workspace-menu-item-recent-workspace-1')).toBeInTheDocument(); + }); + + it('should be able to display empty state when seach is not found', () => { + coreStartMock.workspaces.workspaceList$.next([ + { id: 'workspace-1', name: 'workspace 1', features: [] }, + { id: 'test-2', name: 'test 2', features: [] }, + ]); + jest + .spyOn(recentWorkspaceManager, 'getRecentWorkspaces') + .mockReturnValue([{ id: 'workspace-1', timestamp: 1234567890 }]); + render(); + + const selectButton = screen.getByTestId('workspace-select-button'); + fireEvent.click(selectButton); + + const searchInput = screen.getByRole('searchbox'); + fireEvent.change(searchInput, { target: { value: 'noitems' } }); + expect(screen.getByText(/no workspace available/i)).toBeInTheDocument(); + }); + + it('should display current workspace name, use case name and associated icon', () => { coreStartMock.workspaces.currentWorkspace$.next({ id: 'workspace-1', name: 'workspace 1', @@ -102,6 +146,7 @@ describe('', () => { fireEvent.click(screen.getByTestId('workspace-select-button')); expect(screen.getByTestId('workspace-menu-current-workspace-name')).toBeInTheDocument(); expect(screen.getByTestId('workspace-menu-current-use-case')).toBeInTheDocument(); + expect(screen.getByTestId('current-workspace-icon-wsObservability')).toBeInTheDocument(); expect(screen.getByText('Observability')).toBeInTheDocument(); }); @@ -155,28 +200,6 @@ describe('', () => { }); }); - it('should navigate to workspace management page', () => { - coreStartMock.workspaces.currentWorkspace$.next({ - id: 'workspace-1', - name: 'workspace 1', - features: ['use-case-observability'], - }); - const navigateToWorkspaceDetail = jest.spyOn(workspaceUtils, 'navigateToWorkspaceDetail'); - render(); - - fireEvent.click(screen.getByTestId('workspace-select-button')); - const button = screen.getByText(/Manage workspace/i); - fireEvent.click(button); - expect(navigateToWorkspaceDetail).toBeCalled(); - }); - - it('should navigate to workspaces management page', () => { - render(); - fireEvent.click(screen.getByTestId('workspace-select-button')); - fireEvent.click(screen.getByText(/manage workspaces/i)); - expect(coreStartMock.application.navigateToApp).toHaveBeenCalledWith('workspace_list'); - }); - it('should navigate to create workspace page', () => { render(); fireEvent.click(screen.getByTestId('workspace-select-button')); @@ -188,7 +211,7 @@ describe('', () => { render(); fireEvent.click(screen.getByTestId('workspace-select-button')); - fireEvent.click(screen.getByText(/View all/i)); + fireEvent.click(screen.getByText(/manage/i)); expect(coreStartMock.application.navigateToApp).toHaveBeenCalledWith('workspace_list'); }); @@ -203,7 +226,7 @@ describe('', () => { render(); fireEvent.click(screen.getByTestId('workspace-select-button')); - expect(screen.getByText(/View all/i)).toBeInTheDocument(); + expect(screen.queryByText(/manage/i)).not.toBeInTheDocument(); expect(screen.queryByText(/create workspaces/i)).toBeNull(); }); }); diff --git a/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx b/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx index d70b6c4419a0..bf36fc3844ff 100644 --- a/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx +++ b/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx @@ -9,21 +9,20 @@ import { useObservable } from 'react-use'; import { EuiText, EuiPanel, + EuiButton, EuiPopover, - EuiToolTip, - EuiFlexItem, - EuiFlexGroup, - EuiSmallButtonEmpty, - EuiSmallButton, EuiButtonIcon, + EuiFlexItem, EuiIcon, + EuiFlexGroup, + EuiHorizontalRule, + EuiButtonEmpty, } from '@elastic/eui'; import { BehaviorSubject } from 'rxjs'; import { WORKSPACE_CREATE_APP_ID, WORKSPACE_LIST_APP_ID } from '../../../common/constants'; import { CoreStart, WorkspaceObject } from '../../../../../core/public'; import { getFirstUseCaseOfFeatureConfigs } from '../../utils'; import { WorkspaceUseCase } from '../../types'; -import { navigateToWorkspaceDetail } from '../utils/workspace'; import { validateWorkspaceColor } from '../../../common/utils'; import { WorkspacePickerContent } from '../workspace_picker_content/workspace_picker_content'; @@ -35,16 +34,8 @@ const createWorkspaceButton = i18n.translate('workspace.menu.button.createWorksp defaultMessage: 'Create workspace', }); -const viewAllButton = i18n.translate('workspace.menu.button.viewAll', { - defaultMessage: 'View all', -}); - -const manageWorkspaceButton = i18n.translate('workspace.menu.button.manageWorkspace', { - defaultMessage: 'Manage workspace', -}); - const manageWorkspacesButton = i18n.translate('workspace.menu.button.manageWorkspaces', { - defaultMessage: 'Manage workspaces', + defaultMessage: 'Manage', }); const getValidWorkspaceColor = (color?: string) => @@ -97,8 +88,9 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => { closePopover={closePopover} panelPaddingSize="s" anchorPosition="downCenter" + repositionOnScroll={true} > - + { <> - - - - {currentWorkspaceName} - - - - - {getUseCase(currentWorkspace)?.title ?? ''} - - - { - closePopover(); - navigateToWorkspaceDetail(coreStart, currentWorkspace.id); - }} + + {currentWorkspaceName} + - {manageWorkspaceButton} - + {getUseCase(currentWorkspace)?.title ?? ''} + ) : ( <> - + - {currentWorkspaceName} - - - { - closePopover(); - coreStart.application.navigateToApp(WORKSPACE_LIST_APP_ID); - }} - > - {manageWorkspacesButton} - + {currentWorkspaceName} )} - + + setPopover(false)} /> - - - - { - closePopover(); - coreStart.application.navigateToApp(WORKSPACE_LIST_APP_ID); - }} - > - {viewAllButton} - - - {isDashboardAdmin && ( + + {isDashboardAdmin && ( + + + + + { + closePopover(); + coreStart.application.navigateToApp(WORKSPACE_LIST_APP_ID); + }} + > + {manageWorkspacesButton} + + + - { @@ -197,12 +180,12 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => { coreStart.application.navigateToApp(WORKSPACE_CREATE_APP_ID); }} > - {createWorkspaceButton} - + {createWorkspaceButton} + - )} - - + + + )} ); }; diff --git a/src/plugins/workspace/public/components/workspace_picker_content/workspace_picker_content.tsx b/src/plugins/workspace/public/components/workspace_picker_content/workspace_picker_content.tsx index 8aa9e8e97823..72fdd67b7367 100644 --- a/src/plugins/workspace/public/components/workspace_picker_content/workspace_picker_content.tsx +++ b/src/plugins/workspace/public/components/workspace_picker_content/workspace_picker_content.tsx @@ -2,13 +2,22 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ - import { i18n } from '@osd/i18n'; -import React, { useMemo } from 'react'; +import React, { useMemo, useState } from 'react'; import { useObservable } from 'react-use'; -import { EuiTitle, EuiListGroup, EuiListGroupItem, EuiIcon } from '@elastic/eui'; +import { + EuiTitle, + EuiIcon, + EuiPanel, + EuiSpacer, + EuiText, + EuiFieldSearch, + EuiListGroup, + EuiListGroupItem, + EuiEmptyPrompt, +} from '@elastic/eui'; + import { BehaviorSubject } from 'rxjs'; -import { MAX_WORKSPACE_PICKER_NUM } from '../../../common/constants'; import { CoreStart, WorkspaceObject } from '../../../../../core/public'; import { recentWorkspaceManager } from '../../recent_workspace_manager'; import { WorkspaceUseCase } from '../../types'; @@ -19,6 +28,10 @@ const allWorkspacesTitle = i18n.translate('workspace.menu.title.allWorkspaces', defaultMessage: 'All workspaces', }); +const searchFieldPlaceholder = i18n.translate('workspace.menu.search.placeholder', { + defaultMessage: 'Search workspace name', +}); + const recentWorkspacesTitle = i18n.translate('workspace.menu.title.recentWorkspaces', { defaultMessage: 'Recent workspaces', }); @@ -38,20 +51,39 @@ export const WorkspacePickerContent = ({ onClickWorkspace, }: Props) => { const workspaceList = useObservable(coreStart.workspaces.workspaceList$, []); + const isDashboardAdmin = coreStart.application.capabilities?.dashboards?.isDashboardAdmin; const availableUseCases = useObservable(registeredUseCases$, []); + const [search, setSearch] = useState(''); - const filteredWorkspaceList = useMemo(() => { - return workspaceList.slice(0, MAX_WORKSPACE_PICKER_NUM); - }, [workspaceList]); - - const filteredRecentWorkspaces = useMemo(() => { + const recentWorkspaces = useMemo(() => { return recentWorkspaceManager .getRecentWorkspaces() .map((workspace) => workspaceList.find((ws) => ws.id === workspace.id)) - .filter((workspace): workspace is WorkspaceObject => workspace !== undefined) - .slice(0, MAX_WORKSPACE_PICKER_NUM); + .filter((workspace): workspace is WorkspaceObject => workspace !== undefined); }, [workspaceList]); + const queryFromList = ({ list, query }: { list: WorkspaceObject[]; query: string }) => { + if (!list || list.length === 0) { + return []; + } + + if (query && query.trim() !== '') { + const normalizedQuery = query.toLowerCase(); + const result = list.filter((item) => item.name.toLowerCase().includes(normalizedQuery)); + return result; + } + + return list; + }; + + const queriedWorkspace = useMemo(() => { + return queryFromList({ list: workspaceList, query: search }); + }, [workspaceList, search]); + + const queriedRecentWorkspace = useMemo(() => { + return queryFromList({ list: recentWorkspaces, query: search }); + }, [recentWorkspaces, search]); + const getUseCase = (workspace: WorkspaceObject) => { if (!workspace.features) { return; @@ -60,10 +92,44 @@ export const WorkspacePickerContent = ({ return availableUseCases.find((useCase) => useCase.id === useCaseId); }; + const getEmptyStatePrompt = () => { + return ( + +

+ {i18n.translate('workspace.picker.empty.state.title', { + defaultMessage: 'No workspace available', + })} +

+ + } + body={ + +

+ {isDashboardAdmin + ? i18n.translate('workspace.picker.empty.state.description.admin', { + defaultMessage: 'Create a workspace to get start', + }) + : i18n.translate('workspace.picker.empty.state.description.noAdmin', { + defaultMessage: + 'Contact your administrator to create a workspace or to be added to an existing one', + })} +

+ + } + /> + ); + }; + const getWorkspaceListGroup = (filterWorkspaceList: WorkspaceObject[], itemType: string) => { const listItems = filterWorkspaceList.map((workspace: WorkspaceObject) => { const useCase = getUseCase(workspace); const useCaseURL = getUseCaseUrl(useCase, workspace, coreStart.application, coreStart.http); + return ( - -

{itemType === 'all' ? allWorkspacesTitle : recentWorkspacesTitle}

- - } - /> - {listItems} - + <> + +

{itemType === 'all' ? allWorkspacesTitle : recentWorkspacesTitle}

+ + + + {listItems} + + + ); }; return ( <> - {filteredRecentWorkspaces.length > 0 && - getWorkspaceListGroup(filteredRecentWorkspaces, 'recent')} - {filteredWorkspaceList.length > 0 && getWorkspaceListGroup(filteredWorkspaceList, 'all')} + setSearch(e.target.value)} + placeholder={searchFieldPlaceholder} + /> + + + + {queriedRecentWorkspace.length > 0 && + getWorkspaceListGroup(queriedRecentWorkspace, 'recent')} + + {queriedWorkspace.length > 0 && getWorkspaceListGroup(queriedWorkspace, 'all')} + + {queriedWorkspace.length === 0 && getEmptyStatePrompt()} + ); }; From bc49b8cd87913958854f09394441e3a12cb5a409 Mon Sep 17 00:00:00 2001 From: yuboluo Date: Thu, 5 Sep 2024 23:52:22 +0800 Subject: [PATCH 07/12] [Data Source]Add data source permission wrapper and dataSourceAdmin role (#7959) * Add data source permission wrapper Signed-off-by: yubonluo * Changeset file for PR #7959 created/updated * optimize the config schema Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * add some coments and optimize the logic Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * add unit tests Signed-off-by: yubonluo * fix test error Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * Move some logic to workspace wrapper Signed-off-by: yubonluo * delete useless code Signed-off-by: yubonluo * delete useless code Signed-off-by: yubonluo --------- Signed-off-by: yubonluo Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/7959.yml | 2 + config/opensearch_dashboards.yml | 4 + .../service/saved_objects_client.test.js | 24 -- .../service/saved_objects_client.ts | 27 +- src/core/server/utils/auth_info.test.ts | 63 ++++ src/core/server/utils/auth_info.ts | 45 +++ src/core/server/utils/index.ts | 1 + src/core/server/utils/workspace.test.ts | 2 + src/core/server/utils/workspace.ts | 6 +- .../data_source_management/common/index.ts | 3 + src/plugins/data_source_management/config.ts | 5 + .../data_source_management/server/plugin.ts | 44 ++- ...a_source_permission_client_wrapper.test.ts | 353 ++++++++++++++++++ .../data_source_premission_client_wrapper.ts | 162 ++++++++ .../server/permission_control/client.test.ts | 2 +- .../server/permission_control/client.ts | 2 +- src/plugins/workspace/server/plugin.test.ts | 8 +- ...space_saved_objects_client_wrapper.test.ts | 2 +- ...space_saved_objects_client_wrapper.test.ts | 105 +++++- .../workspace_saved_objects_client_wrapper.ts | 45 ++- src/plugins/workspace/server/types.ts | 5 - src/plugins/workspace/server/utils.test.ts | 54 --- src/plugins/workspace/server/utils.ts | 36 -- 23 files changed, 846 insertions(+), 154 deletions(-) create mode 100644 changelogs/fragments/7959.yml create mode 100644 src/core/server/utils/auth_info.test.ts create mode 100644 src/core/server/utils/auth_info.ts create mode 100644 src/plugins/data_source_management/server/saved_objects/data_source_permission_client_wrapper.test.ts create mode 100644 src/plugins/data_source_management/server/saved_objects/data_source_premission_client_wrapper.ts diff --git a/changelogs/fragments/7959.yml b/changelogs/fragments/7959.yml new file mode 100644 index 000000000000..9cf096121413 --- /dev/null +++ b/changelogs/fragments/7959.yml @@ -0,0 +1,2 @@ +feat: +- [Data source] Add data source permission wrapper and dataSourceAdmin role ([#7959](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7959)) \ No newline at end of file diff --git a/config/opensearch_dashboards.yml b/config/opensearch_dashboards.yml index 47de509b4483..349b002c9a1c 100644 --- a/config/opensearch_dashboards.yml +++ b/config/opensearch_dashboards.yml @@ -327,6 +327,10 @@ # "all": The data source can be managed by all users. Default to "all". # data_source_management.manageableBy: "all" +# Set the backend roles in groups, whoever has the backend roles defined in this config will be regard as dataSourceAdmin. +# DataSource Admin will have the access to all the data source saved objects inside OpenSearch Dashboards by api. +# data_source_management.dataSourceAdmin.groups: ["data_source_management"] + # Set the value of this setting to false to hide the help menu link to the OpenSearch Dashboards user survey # opensearchDashboards.survey.url: "https://survey.opensearch.org" diff --git a/src/core/server/saved_objects/service/saved_objects_client.test.js b/src/core/server/saved_objects/service/saved_objects_client.test.js index aa597f98379a..8830220a3e3a 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.test.js +++ b/src/core/server/saved_objects/service/saved_objects_client.test.js @@ -246,30 +246,6 @@ test(`#deleteFromWorkspaces Should use update if there is existing workspaces`, }); }); -test(`#deleteFromWorkspaces Should use overwrite create if there is no existing workspaces`, async () => { - const returnValue = Symbol(); - const create = jest.fn(); - const mockRepository = { - get: jest.fn().mockResolvedValue({ - workspaces: [], - }), - update: jest.fn().mockResolvedValue(returnValue), - create, - }; - const client = new SavedObjectsClient(mockRepository); - - const type = Symbol(); - const id = Symbol(); - const workspaces = ['id1']; - await client.deleteFromWorkspaces(type, id, workspaces); - expect(mockRepository.get).toHaveBeenCalledWith(type, id, {}); - expect(mockRepository.create).toHaveBeenCalledWith( - type, - {}, - { id, overwrite: true, permissions: undefined, version: undefined } - ); -}); - test(`#deleteFromWorkspaces should throw error if no workspaces passed`, () => { const mockRepository = {}; const client = new SavedObjectsClient(mockRepository); diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index f17e6e35f0bd..741fd5f5f874 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -488,28 +488,11 @@ export class SavedObjectsClient { const newWorkspaces = existingWorkspaces.filter((item) => { return targetWorkspaces.indexOf(item) === -1; }); - if (newWorkspaces.length > 0) { - return await this.update(type, id, object.attributes, { - ...options, - workspaces: newWorkspaces, - version: object.version, - }); - } else { - // If there is no workspaces assigned, will create object with overwrite to delete workspace property. - return await this.create( - type, - { - ...object.attributes, - }, - { - ...options, - id, - permissions: object.permissions, - overwrite: true, - version: object.version, - } - ); - } + return await this.update(type, id, object.attributes, { + ...options, + workspaces: newWorkspaces, + version: object.version, + }); }; /** diff --git a/src/core/server/utils/auth_info.test.ts b/src/core/server/utils/auth_info.test.ts new file mode 100644 index 000000000000..d270f39be1b3 --- /dev/null +++ b/src/core/server/utils/auth_info.test.ts @@ -0,0 +1,63 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { AuthStatus } from '../http/auth_state_storage'; +import { httpServerMock, httpServiceMock } from '../mocks'; +import { getPrincipalsFromRequest } from './auth_info'; + +describe('utils', () => { + const mockAuth = httpServiceMock.createAuth(); + it('should return empty map when request do not have authentication', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + mockAuth.get.mockReturnValueOnce({ + status: AuthStatus.unknown, + state: { + authInfo: { + user_name: 'bar', + backend_roles: ['foo'], + }, + }, + }); + const result = getPrincipalsFromRequest(mockRequest, mockAuth); + expect(result).toEqual({}); + }); + + it('should return normally when request has authentication', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + mockAuth.get.mockReturnValueOnce({ + status: AuthStatus.authenticated, + state: { + authInfo: { + user_name: 'bar', + backend_roles: ['foo'], + }, + }, + }); + const result = getPrincipalsFromRequest(mockRequest, mockAuth); + expect(result.users).toEqual(['bar']); + expect(result.groups).toEqual(['foo']); + }); + + it('should throw error when request is not authenticated', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + mockAuth.get.mockReturnValueOnce({ + status: AuthStatus.unauthenticated, + state: {}, + }); + expect(() => getPrincipalsFromRequest(mockRequest, mockAuth)).toThrow('NOT_AUTHORIZED'); + }); + + it('should throw error when authentication status is not expected', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + mockAuth.get.mockReturnValueOnce({ + // @ts-expect-error + status: 'foo', + state: {}, + }); + expect(() => getPrincipalsFromRequest(mockRequest, mockAuth)).toThrow( + 'UNEXPECTED_AUTHORIZATION_STATUS' + ); + }); +}); diff --git a/src/core/server/utils/auth_info.ts b/src/core/server/utils/auth_info.ts new file mode 100644 index 000000000000..7d965ad05ca9 --- /dev/null +++ b/src/core/server/utils/auth_info.ts @@ -0,0 +1,45 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { AuthStatus } from '../http/auth_state_storage'; +import { OpenSearchDashboardsRequest } from '../http/router'; +import { HttpAuth } from '../http/types'; +import { PrincipalType, Principals } from '../saved_objects/permission_control/acl'; + +export interface AuthInfo { + backend_roles?: string[]; + user_name?: string; +} + +export const getPrincipalsFromRequest = ( + request: OpenSearchDashboardsRequest, + auth?: HttpAuth +): Principals => { + const payload: Principals = {}; + const authInfoResp = auth?.get(request); + if (authInfoResp?.status === AuthStatus.unknown) { + /** + * Login user have access to all the workspaces when no authentication is presented. + */ + return payload; + } + + if (authInfoResp?.status === AuthStatus.authenticated) { + const authState = authInfoResp?.state as { authInfo: AuthInfo } | null; + if (authState?.authInfo?.backend_roles) { + payload[PrincipalType.Groups] = authState.authInfo.backend_roles; + } + if (authState?.authInfo?.user_name) { + payload[PrincipalType.Users] = [authState.authInfo.user_name]; + } + return payload; + } + + if (authInfoResp?.status === AuthStatus.unauthenticated) { + throw new Error('NOT_AUTHORIZED'); + } + + throw new Error('UNEXPECTED_AUTHORIZATION_STATUS'); +}; diff --git a/src/core/server/utils/index.ts b/src/core/server/utils/index.ts index a20b8c4c4e5b..64f8379a46e2 100644 --- a/src/core/server/utils/index.ts +++ b/src/core/server/utils/index.ts @@ -32,5 +32,6 @@ export * from './crypto'; export * from './from_root'; export * from './package_json'; export * from './streams'; +export { getPrincipalsFromRequest } from './auth_info'; export { getWorkspaceIdFromUrl, cleanWorkspaceId } from '../../utils'; export { updateWorkspaceState, getWorkspaceState } from './workspace'; diff --git a/src/core/server/utils/workspace.test.ts b/src/core/server/utils/workspace.test.ts index 19f8bad4f866..702542715ab2 100644 --- a/src/core/server/utils/workspace.test.ts +++ b/src/core/server/utils/workspace.test.ts @@ -12,10 +12,12 @@ describe('updateWorkspaceState', () => { updateWorkspaceState(requestMock, { requestWorkspaceId: 'foo', isDashboardAdmin: true, + isDataSourceAdmin: true, }); expect(getWorkspaceState(requestMock)).toEqual({ requestWorkspaceId: 'foo', isDashboardAdmin: true, + isDataSourceAdmin: true, }); }); }); diff --git a/src/core/server/utils/workspace.ts b/src/core/server/utils/workspace.ts index 89f2b7975964..efc75d96a1c2 100644 --- a/src/core/server/utils/workspace.ts +++ b/src/core/server/utils/workspace.ts @@ -8,6 +8,7 @@ import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router'; export interface WorkspaceState { requestWorkspaceId?: string; isDashboardAdmin?: boolean; + isDataSourceAdmin?: boolean; } /** @@ -29,10 +30,13 @@ export const updateWorkspaceState = ( }; }; +// TODO: Move isDataSourceAdmin and isDashboardAdmin out of WorkspaceState and this change is planned for version 2.18 export const getWorkspaceState = (request: OpenSearchDashboardsRequest): WorkspaceState => { - const { requestWorkspaceId, isDashboardAdmin } = ensureRawRequest(request).app as WorkspaceState; + const { requestWorkspaceId, isDashboardAdmin, isDataSourceAdmin } = ensureRawRequest(request) + .app as WorkspaceState; return { requestWorkspaceId, isDashboardAdmin, + isDataSourceAdmin, }; }; diff --git a/src/plugins/data_source_management/common/index.ts b/src/plugins/data_source_management/common/index.ts index 2b9b3a4454bf..aba2f1208e82 100644 --- a/src/plugins/data_source_management/common/index.ts +++ b/src/plugins/data_source_management/common/index.ts @@ -7,3 +7,6 @@ export const PLUGIN_ID = 'dataSourceManagement'; export const PLUGIN_NAME = 'Data sources'; export const DEFAULT_DATA_SOURCE_UI_SETTINGS_ID = 'defaultDataSource'; export * from './types'; +export const DATA_SOURCE_PERMISSION_CLIENT_WRAPPER_ID = 'data-source-permission'; +// Run data source permission wrapper behind all other wrapper. +export const ORDER_FOR_DATA_SOURCE_PERMISSION_WRAPPER = 50; diff --git a/src/plugins/data_source_management/config.ts b/src/plugins/data_source_management/config.ts index 1a56a126a943..1cc93c85b44e 100644 --- a/src/plugins/data_source_management/config.ts +++ b/src/plugins/data_source_management/config.ts @@ -10,6 +10,11 @@ export const configSchema = schema.object({ [schema.literal('all'), schema.literal('dashboard_admin'), schema.literal('none')], { defaultValue: 'all' } ), + dataSourceAdmin: schema.object({ + groups: schema.arrayOf(schema.string(), { + defaultValue: [], + }), + }), }); export type ConfigSchema = TypeOf; diff --git a/src/plugins/data_source_management/server/plugin.ts b/src/plugins/data_source_management/server/plugin.ts index de8dcf74cf96..dad9242e04f0 100644 --- a/src/plugins/data_source_management/server/plugin.ts +++ b/src/plugins/data_source_management/server/plugin.ts @@ -21,8 +21,17 @@ import { DataSourceManagementPluginSetup, DataSourceManagementPluginStart } from import { OpenSearchDataSourceManagementPlugin } from './adaptors/opensearch_data_source_management_plugin'; import { PPLPlugin } from './adaptors/ppl_plugin'; import { ConfigSchema } from '../config'; -import { getWorkspaceState } from '../../../../src/core/server/utils'; -import { ManageableBy } from '../common'; +import { + getPrincipalsFromRequest, + getWorkspaceState, + updateWorkspaceState, +} from '../../../../src/core/server/utils'; +import { + DATA_SOURCE_PERMISSION_CLIENT_WRAPPER_ID, + ManageableBy, + ORDER_FOR_DATA_SOURCE_PERMISSION_WRAPPER, +} from '../common'; +import { DataSourcePermissionClientWrapper } from './saved_objects/data_source_premission_client_wrapper'; export interface DataSourceManagementPluginDependencies { dataSource: DataSourcePluginSetup; @@ -33,6 +42,35 @@ export class DataSourceManagementPlugin private readonly config$: Observable; private readonly logger: Logger; + private setupDataSourcePermission(core: CoreSetup, config: ConfigSchema) { + core.http.registerOnPostAuth(async (request, response, toolkit) => { + let groups: string[]; + const [coreStart] = await core.getStartServices(); + + try { + ({ groups = [] } = getPrincipalsFromRequest(request, coreStart.http.auth)); + } catch (e) { + return toolkit.next(); + } + + const configGroups = config.dataSourceAdmin.groups; + const isDataSourceAdmin = configGroups.some((configGroup) => groups.includes(configGroup)); + updateWorkspaceState(request, { + isDataSourceAdmin, + }); + return toolkit.next(); + }); + + const dataSourcePermissionWrapper = new DataSourcePermissionClientWrapper(config.manageableBy); + + // Add data source permission client wrapper factory + core.savedObjects.addClientWrapper( + ORDER_FOR_DATA_SOURCE_PERMISSION_WRAPPER, + DATA_SOURCE_PERMISSION_CLIENT_WRAPPER_ID, + dataSourcePermissionWrapper.wrapperFactory + ); + } + constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get(); this.config$ = initializerContext.config.create(); @@ -82,6 +120,8 @@ export class DataSourceManagementPlugin if (dataSourceEnabled) { dataSource.registerCustomApiSchema(PPLPlugin); dataSource.registerCustomApiSchema(OpenSearchDataSourceManagementPlugin); + + this.setupDataSourcePermission(core, config); } // @ts-ignore core.http.registerRouteHandlerContext( diff --git a/src/plugins/data_source_management/server/saved_objects/data_source_permission_client_wrapper.test.ts b/src/plugins/data_source_management/server/saved_objects/data_source_permission_client_wrapper.test.ts new file mode 100644 index 000000000000..280ee32c183e --- /dev/null +++ b/src/plugins/data_source_management/server/saved_objects/data_source_permission_client_wrapper.test.ts @@ -0,0 +1,353 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as utils from '../../../../core/server/utils/workspace'; +import { coreMock, httpServerMock, savedObjectsClientMock } from '../../../../core/server/mocks'; +import { DataSourcePermissionClientWrapper } from './data_source_premission_client_wrapper'; +import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../data_source/common'; +import { ManageableBy } from '../../common'; + +describe('DataSourcePermissionClientWrapper', () => { + const requestHandlerContext = coreMock.createRequestHandlerContext(); + const requestMock = httpServerMock.createOpenSearchDashboardsRequest(); + + const attributes = { + title: 'data-source', + description: 'jest testing', + endpoint: 'https://test.com', + auth: { type: 'no_auth' }, + workspaces: ['workspace-1'], + }; + + const dataSource = { + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + attributes, + }; + const dashboard = { + type: 'dashboard', + attributes: {}, + }; + + const errorMessage = 'You have no permission to perform this operation'; + + describe('Data source is managed by dashboard admin', () => { + describe('user is not dashboard admin', () => { + jest.spyOn(utils, 'getWorkspaceState').mockReturnValue({ isDashboardAdmin: false }); + const mockedClient = savedObjectsClientMock.create(); + const wrapperInstance = new DataSourcePermissionClientWrapper(ManageableBy.DashboardAdmin); + const wrapperClient = wrapperInstance.wrapperFactory({ + client: mockedClient, + typeRegistry: requestHandlerContext.savedObjects.typeRegistry, + request: requestMock, + }); + + it('should not create data source when user is not dashboard admin', async () => { + let errorCatch; + try { + await wrapperClient.create(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes, {}); + } catch (e) { + errorCatch = e; + } + expect(errorCatch.message).toEqual(errorMessage); + }); + + it('should not bulk create data source when user is not admin', async () => { + const mockCreateObjects = [dataSource, dashboard]; + const result = await wrapperClient.bulkCreate(mockCreateObjects); + expect(result.saved_objects[0].error?.message).toEqual(errorMessage); + }); + + it('should not update data source when user is not admin', async () => { + let errorCatch; + try { + await wrapperClient.update(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id', {}); + } catch (e) { + errorCatch = e; + } + expect(errorCatch.message).toEqual(errorMessage); + }); + + it('should not bulk update data source when user is not admin', async () => { + const mockCreateObjects = [ + { ...dataSource, id: 'data-source-id' }, + { ...dashboard, id: 'dashboard-id' }, + ]; + const result = await wrapperClient.bulkUpdate(mockCreateObjects); + expect(result.saved_objects[0].error?.message).toEqual(errorMessage); + }); + + it('should not delete data source when user is not admin', async () => { + let errorCatch; + try { + await wrapperClient.delete(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id'); + } catch (e) { + errorCatch = e; + } + expect(errorCatch.message).toEqual(errorMessage); + }); + }); + describe('user is osd admin', () => { + jest.spyOn(utils, 'getWorkspaceState').mockReturnValue({ isDashboardAdmin: true }); + const mockedClient = savedObjectsClientMock.create(); + const wrapperInstance = new DataSourcePermissionClientWrapper(ManageableBy.DashboardAdmin); + const wrapperClient = wrapperInstance.wrapperFactory({ + client: mockedClient, + typeRegistry: requestHandlerContext.savedObjects.typeRegistry, + request: requestMock, + }); + + it('should create data source when user is admin', async () => { + await wrapperClient.create(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes, {}); + expect(mockedClient.create).toBeCalledWith(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes, {}); + }); + + it('should bulk create data source when user is admin', async () => { + const mockCreateObjects = [dataSource]; + await wrapperClient.bulkCreate(mockCreateObjects, { overwrite: true }); + expect(mockedClient.bulkCreate).toBeCalledWith(mockCreateObjects, { overwrite: true }); + }); + + it('should update data source when user is admin', async () => { + await wrapperClient.update(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id', {}); + expect(mockedClient.update).toBeCalledWith( + DATA_SOURCE_SAVED_OBJECT_TYPE, + 'data-source-id', + {} + ); + }); + + it('should bulk update data source when user is admin', async () => { + const mockUpdateObjects = [ + { + ...dataSource, + id: 'data-source-id', + }, + ]; + await wrapperClient.bulkUpdate(mockUpdateObjects, {}); + expect(mockedClient.bulkUpdate).toBeCalledWith(mockUpdateObjects, {}); + }); + + it('should delete data source', async () => { + await wrapperClient.delete(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id'); + expect(mockedClient.delete).toBeCalledWith(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id'); + }); + }); + describe('any user is osd admin when osd admin is not configured', () => { + jest.spyOn(utils, 'getWorkspaceState').mockReturnValue({}); + const mockedClient = savedObjectsClientMock.create(); + const wrapperInstance = new DataSourcePermissionClientWrapper(ManageableBy.DashboardAdmin); + const wrapperClient = wrapperInstance.wrapperFactory({ + client: mockedClient, + typeRegistry: requestHandlerContext.savedObjects.typeRegistry, + request: requestMock, + }); + + it('should create data source when user is admin', async () => { + await wrapperClient.create(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes, {}); + expect(mockedClient.create).toBeCalledWith(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes, {}); + }); + + it('should bulk create data source when user is admin', async () => { + const mockCreateObjects = [dataSource]; + await wrapperClient.bulkCreate(mockCreateObjects, { overwrite: true }); + expect(mockedClient.bulkCreate).toBeCalledWith(mockCreateObjects, { overwrite: true }); + }); + + it('should update data source when user is admin', async () => { + await wrapperClient.update(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id', {}); + expect(mockedClient.update).toBeCalledWith( + DATA_SOURCE_SAVED_OBJECT_TYPE, + 'data-source-id', + {} + ); + }); + + it('should bulk update data source when user is admin', async () => { + const mockUpdateObjects = [ + { + ...dataSource, + id: 'data-source-id', + }, + ]; + await wrapperClient.bulkUpdate(mockUpdateObjects, {}); + expect(mockedClient.bulkUpdate).toBeCalledWith(mockUpdateObjects, {}); + }); + + it('should delete data source', async () => { + jest.spyOn(utils, 'getWorkspaceState').mockReturnValue({}); + await wrapperClient.delete(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id'); + expect(mockedClient.delete).toBeCalledWith(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id'); + }); + }); + }); + + describe('Data source is managed by none', () => { + jest.spyOn(utils, 'getWorkspaceState').mockReturnValue({ isDashboardAdmin: false }); + const mockedClient = { + ...savedObjectsClientMock.create(), + get: jest.fn().mockImplementation(async (id) => { + return { + id, + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + attributes, + }; + }), + }; + const wrapperInstance = new DataSourcePermissionClientWrapper(ManageableBy.None); + const wrapperClient = wrapperInstance.wrapperFactory({ + client: mockedClient, + typeRegistry: requestHandlerContext.savedObjects.typeRegistry, + request: requestMock, + }); + + it('should not create data source', async () => { + let errorCatch; + try { + await wrapperClient.create(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes, {}); + } catch (e) { + errorCatch = e; + } + expect(errorCatch.message).toEqual(errorMessage); + + await wrapperClient.create('dashboard', {}, {}); + expect(mockedClient.create).toBeCalledWith('dashboard', {}, {}); + }); + + it('should not bulk create data source', async () => { + const mockCreateObjects = [dataSource, dashboard]; + const result = await wrapperClient.bulkCreate(mockCreateObjects); + expect(result.saved_objects[0].error?.message).toEqual(errorMessage); + }); + + it('should not update data source', async () => { + let errorCatch; + try { + await wrapperClient.update(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id', {}); + } catch (e) { + errorCatch = e; + } + expect(errorCatch.message).toEqual(errorMessage); + + await wrapperClient.update('dashboard', 'dashboard-id', {}); + expect(mockedClient.update).toBeCalledWith('dashboard', 'dashboard-id', {}, {}); + }); + + it('should not bulk update data source', async () => { + const mockCreateObjects = [ + { ...dataSource, id: 'data-source-id' }, + { ...dashboard, id: 'dashboard-id' }, + ]; + const result = await wrapperClient.bulkUpdate(mockCreateObjects); + expect(result.saved_objects[0].error?.message).toEqual(errorMessage); + }); + + it('should not delete data source', async () => { + let errorCatch; + try { + await wrapperClient.delete(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id'); + } catch (e) { + errorCatch = e; + } + expect(errorCatch.message).toEqual(errorMessage); + + await wrapperClient.delete('dashboard', 'dashboard-id'); + expect(mockedClient.delete).toBeCalledWith('dashboard', 'dashboard-id', {}); + }); + }); + + describe('User is data source admin', () => { + jest.spyOn(utils, 'getWorkspaceState').mockReturnValue({ isDataSourceAdmin: true }); + const mockedClient = savedObjectsClientMock.create(); + const wrapperInstance = new DataSourcePermissionClientWrapper(ManageableBy.None); + const wrapperClient = wrapperInstance.wrapperFactory({ + client: mockedClient, + typeRegistry: requestHandlerContext.savedObjects.typeRegistry, + request: requestMock, + }); + + it('should create data source', async () => { + await wrapperClient.create(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes, {}); + expect(mockedClient.create).toBeCalledWith(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes, {}); + }); + + it('should bulk create data source', async () => { + const mockCreateObjects = [dataSource]; + await wrapperClient.bulkCreate(mockCreateObjects, { overwrite: true }); + expect(mockedClient.bulkCreate).toBeCalledWith(mockCreateObjects, { overwrite: true }); + }); + + it('should update data source', async () => { + await wrapperClient.update(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id', {}); + expect(mockedClient.update).toBeCalledWith( + DATA_SOURCE_SAVED_OBJECT_TYPE, + 'data-source-id', + {} + ); + }); + + it('should bulk update data source', async () => { + const mockUpdateObjects = [ + { + ...dataSource, + id: 'data-source-id', + }, + ]; + await wrapperClient.bulkUpdate(mockUpdateObjects, {}); + expect(mockedClient.bulkUpdate).toBeCalledWith(mockUpdateObjects, {}); + }); + + it('should delete data source', async () => { + await wrapperClient.delete(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id'); + expect(mockedClient.delete).toBeCalledWith(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id'); + }); + }); + + describe('Data source is managed by all', () => { + jest.spyOn(utils, 'getWorkspaceState').mockReturnValue({ isDashboardAdmin: false }); + const mockedClient = savedObjectsClientMock.create(); + const wrapperInstance = new DataSourcePermissionClientWrapper(ManageableBy.All); + const wrapperClient = wrapperInstance.wrapperFactory({ + client: mockedClient, + typeRegistry: requestHandlerContext.savedObjects.typeRegistry, + request: requestMock, + }); + + it('should create data source', async () => { + await wrapperClient.create(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes, {}); + expect(mockedClient.create).toBeCalledWith(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes, {}); + }); + + it('should bulk create data source', async () => { + const mockCreateObjects = [dataSource]; + await wrapperClient.bulkCreate(mockCreateObjects, { overwrite: true }); + expect(mockedClient.bulkCreate).toBeCalledWith(mockCreateObjects, { overwrite: true }); + }); + + it('should update data source', async () => { + await wrapperClient.update(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id', {}); + expect(mockedClient.update).toBeCalledWith( + DATA_SOURCE_SAVED_OBJECT_TYPE, + 'data-source-id', + {} + ); + }); + + it('should bulk update data source', async () => { + const mockUpdateObjects = [ + { + ...dataSource, + id: 'data-source-id', + }, + ]; + await wrapperClient.bulkUpdate(mockUpdateObjects, {}); + expect(mockedClient.bulkUpdate).toBeCalledWith(mockUpdateObjects, {}); + }); + + it('should delete data source', async () => { + await wrapperClient.delete(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id'); + expect(mockedClient.delete).toBeCalledWith(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id'); + }); + }); +}); diff --git a/src/plugins/data_source_management/server/saved_objects/data_source_premission_client_wrapper.ts b/src/plugins/data_source_management/server/saved_objects/data_source_premission_client_wrapper.ts new file mode 100644 index 000000000000..17c979303535 --- /dev/null +++ b/src/plugins/data_source_management/server/saved_objects/data_source_premission_client_wrapper.ts @@ -0,0 +1,162 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { i18n } from '@osd/i18n'; +import { + SavedObjectsBulkCreateObject, + SavedObjectsBulkResponse, + SavedObjectsBulkUpdateObject, + SavedObjectsBulkUpdateOptions, + SavedObjectsBulkUpdateResponse, + SavedObjectsClientWrapperFactory, + SavedObjectsCreateOptions, + SavedObjectsDeleteOptions, + SavedObjectsErrorHelpers, + SavedObjectsUpdateOptions, + SavedObjectsUpdateResponse, +} from '../../../../core/server'; +import { getWorkspaceState } from '../../../../core/server/utils'; +import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../data_source/common'; +import { ManageableBy } from '../../common'; + +/** + * Determine whether the user has the permissions to create, delete, and update data source based on manageableBy/dataSourceAdmin. + * DataSourceAdmin user has all permissions. + * If manageableBy is all, any user has permissions. + * If manageableBy is none, any user has no permissions. + * If manageableBy is dashboard_admin, only OSD admin has permissions. + */ +export class DataSourcePermissionClientWrapper { + constructor(private manageableBy: string) {} + + public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { + const { isDashboardAdmin, isDataSourceAdmin } = getWorkspaceState(wrapperOptions.request) || {}; + // If isDashboardAdmin is undefined / true, the user will be dashboard admin + const isDashboardAdminRequest = isDashboardAdmin !== false; + if ( + isDataSourceAdmin || + this.manageableBy === ManageableBy.All || + (this.manageableBy === ManageableBy.DashboardAdmin && isDashboardAdminRequest) + ) { + return wrapperOptions.client; + } + + const createWithManageableBy = async ( + type: string, + attributes: T, + options?: SavedObjectsCreateOptions + ) => { + if (DATA_SOURCE_SAVED_OBJECT_TYPE === type) { + throw this.generatePermissionError(); + } + return await wrapperOptions.client.create(type, attributes, options); + }; + + const bulkCreateWithManageableBy = async ( + objects: Array>, + options?: SavedObjectsCreateOptions + ): Promise> => { + const disallowedSavedObjects: Array> = []; + const allowedSavedObjects: Array> = []; + + objects.forEach((item) => { + if (DATA_SOURCE_SAVED_OBJECT_TYPE === item.type) { + disallowedSavedObjects.push(item); + } else { + allowedSavedObjects.push(item); + } + }); + + const bulkCreateResult = await wrapperOptions.client.bulkCreate(allowedSavedObjects, options); + + // Merge the data source saved objects and real client bulkCreate result. + return { + saved_objects: [ + ...(bulkCreateResult?.saved_objects || []), + ...disallowedSavedObjects.map((item) => ({ + ...item, + error: { + ...this.generatePermissionError().output.payload, + metadata: { isNotOverwritable: true }, + }, + })), + ], + } as SavedObjectsBulkResponse; + }; + + const updateWithManageableBy = async ( + type: string, + id: string, + attributes: Partial, + options: SavedObjectsUpdateOptions = {} + ): Promise> => { + if (DATA_SOURCE_SAVED_OBJECT_TYPE === type) { + throw this.generatePermissionError(); + } + return await wrapperOptions.client.update(type, id, attributes, options); + }; + + const bulkUpdateWithManageableBy = async ( + objects: Array>, + options?: SavedObjectsBulkUpdateOptions + ): Promise> => { + const disallowedSavedObjects: Array> = []; + const allowedSavedObjects: Array> = []; + + objects.forEach((item) => { + if (DATA_SOURCE_SAVED_OBJECT_TYPE === item.type) { + disallowedSavedObjects.push(item); + } else { + allowedSavedObjects.push(item); + } + }); + + const bulkUpdateResult = await wrapperOptions.client.bulkUpdate(allowedSavedObjects, options); + + // Merge the data source saved objects and real client bulkUpdate result. + return { + saved_objects: [ + ...(bulkUpdateResult?.saved_objects || []), + ...disallowedSavedObjects.map((item) => ({ + ...item, + error: { + ...this.generatePermissionError().output.payload, + metadata: { isNotOverwritable: true }, + }, + })), + ], + } as SavedObjectsBulkUpdateResponse; + }; + + const deleteWithManageableBy = async ( + type: string, + id: string, + options: SavedObjectsDeleteOptions = {} + ) => { + if (DATA_SOURCE_SAVED_OBJECT_TYPE === type) { + throw this.generatePermissionError(); + } + return await wrapperOptions.client.delete(type, id, options); + }; + + return { + ...wrapperOptions.client, + create: createWithManageableBy, + bulkCreate: bulkCreateWithManageableBy, + delete: deleteWithManageableBy, + update: updateWithManageableBy, + bulkUpdate: bulkUpdateWithManageableBy, + }; + }; + + private generatePermissionError = () => + SavedObjectsErrorHelpers.decorateForbiddenError( + new Error( + i18n.translate('dashboard.admin.permission.invalidate', { + defaultMessage: 'You have no permission to perform this operation', + }) + ) + ); +} diff --git a/src/plugins/workspace/server/permission_control/client.test.ts b/src/plugins/workspace/server/permission_control/client.test.ts index a585710da54d..4dfb3197b0dd 100644 --- a/src/plugins/workspace/server/permission_control/client.test.ts +++ b/src/plugins/workspace/server/permission_control/client.test.ts @@ -10,7 +10,7 @@ import { httpServiceMock, savedObjectsClientMock, } from '../../../../core/server/mocks'; -import * as utilsExports from '../utils'; +import * as utilsExports from '../../../../core/server/utils/auth_info'; describe('PermissionControl', () => { jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation(() => ({ diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts index 0850690325f1..3d052c3fe2f6 100644 --- a/src/plugins/workspace/server/permission_control/client.ts +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -16,7 +16,7 @@ import { HttpAuth, } from '../../../../core/server'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../../common/constants'; -import { getPrincipalsFromRequest } from '../utils'; +import { getPrincipalsFromRequest } from '../../../../core/server/utils'; export type SavedObjectsPermissionControlContract = Pick< SavedObjectsPermissionControl, diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index 433fb2703f00..b9a402c306f9 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -7,12 +7,14 @@ import { OnPostAuthHandler, OnPreRoutingHandler } from 'src/core/server'; import { coreMock, httpServerMock, uiSettingsServiceMock } from '../../../core/server/mocks'; import { WorkspacePlugin } from './plugin'; import { getWorkspaceState, updateWorkspaceState } from '../../../core/server/utils'; +import * as serverUtils from '../../../core/server/utils/auth_info'; import * as utilsExports from './utils'; import { SavedObjectsPermissionControl } from './permission_control/client'; describe('Workspace server plugin', () => { afterEach(() => { jest.clearAllMocks(); + jest.restoreAllMocks(); }); it('#setup', async () => { @@ -132,7 +134,7 @@ describe('Workspace server plugin', () => { it('with yml config', async () => { jest - .spyOn(utilsExports, 'getPrincipalsFromRequest') + .spyOn(serverUtils, 'getPrincipalsFromRequest') .mockImplementation(() => ({ users: [`user1`] })); jest .spyOn(utilsExports, 'getOSDAdminConfigFromYMLConfig') @@ -150,7 +152,7 @@ describe('Workspace server plugin', () => { }); it('uninstall security plugin', async () => { - jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation(() => ({})); + jest.spyOn(serverUtils, 'getPrincipalsFromRequest').mockImplementation(() => ({})); await workspacePlugin.setup(setupMock); const toolKitMock = httpServerMock.createToolkit(); @@ -164,7 +166,7 @@ describe('Workspace server plugin', () => { }); it('should clear saved objects cache', async () => { - jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation(() => ({})); + jest.spyOn(serverUtils, 'getPrincipalsFromRequest').mockImplementation(() => ({})); const clearSavedObjectsCacheMock = jest .spyOn(SavedObjectsPermissionControl.prototype, 'clearSavedObjectsCache') .mockImplementationOnce(() => {}); diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts index 6aeb2ebc8610..607d55d8d5dd 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts @@ -16,7 +16,7 @@ import { SavedObjectsClientContract, } from '../../../../../core/server'; import { httpServerMock } from '../../../../../../src/core/server/mocks'; -import * as utilsExports from '../../utils'; +import * as utilsExports from '../../../../../core/server/utils/auth_info'; import { updateWorkspaceState } from '../../../../../core/server/utils'; const repositoryKit = (() => { diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts index 85d372b76177..ea105cc21b67 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts @@ -9,8 +9,9 @@ import { WorkspaceSavedObjectsClientWrapper } from './workspace_saved_objects_cl import { httpServerMock } from '../../../../core/server/mocks'; import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../data_source/common'; -const DASHBOARD_ADMIN = 'dashnoard_admin'; -const NO_DASHBOARD_ADMIN = 'no_dashnoard_admin'; +const DASHBOARD_ADMIN = 'dashboard_admin'; +const NO_DASHBOARD_ADMIN = 'no_dashboard_admin'; +const DATASOURCE_ADMIN = 'dataSource_admin'; const generateWorkspaceSavedObjectsClientWrapper = (role = NO_DASHBOARD_ADMIN) => { const savedObjectsStore = [ @@ -107,10 +108,18 @@ const generateWorkspaceSavedObjectsClientWrapper = (role = NO_DASHBOARD_ADMIN) = }; }), deleteByWorkspace: jest.fn(), + addToWorkspaces: jest.fn(), + deleteFromWorkspaces: jest.fn(), }; const requestMock = httpServerMock.createOpenSearchDashboardsRequest(); updateWorkspaceState(requestMock, { requestWorkspaceId: 'mock-request-workspace-id' }); - if (role === DASHBOARD_ADMIN) updateWorkspaceState(requestMock, { isDashboardAdmin: true }); + if (role === DASHBOARD_ADMIN) { + updateWorkspaceState(requestMock, { isDashboardAdmin: true }); + } + if (role === DATASOURCE_ADMIN) { + updateWorkspaceState(requestMock, { isDataSourceAdmin: true }); + } + const wrapperOptions = { client: clientMock, request: requestMock, @@ -600,6 +609,17 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { workspaces: ['workspace-1'], }); }); + + it('should not validate data source when user is data source admin', async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(DATASOURCE_ADMIN); + const result = await wrapper.get('data-source', 'workspace-1-data-source'); + expect(result).toEqual({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + id: 'workspace-1-data-source', + attributes: { title: 'Workspace 1 data source' }, + workspaces: ['workspace-1'], + }); + }); }); describe('bulk get', () => { it("should call permission validate with object's workspace and throw permission error", async () => { @@ -772,6 +792,23 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { workspaces: ['workspace-1'], }); }); + it('should call client.find without ACLSearchParams and workspaceOperator', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper( + DATASOURCE_ADMIN + ); + await wrapper.find({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + }); + expect(clientMock.find).toHaveBeenCalledWith({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + }); + await wrapper.find({ + type: [DATA_SOURCE_SAVED_OBJECT_TYPE], + }); + expect(clientMock.find).toHaveBeenCalledWith({ + type: [DATA_SOURCE_SAVED_OBJECT_TYPE], + }); + }); }); describe('deleteByWorkspace', () => { @@ -912,6 +949,68 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('not-permitted-workspace'); expect(permissionControlMock.validate).not.toHaveBeenCalled(); }); + it('should bypass permission check for call client.addToWorkspaces', async () => { + await wrapper.addToWorkspaces( + DATA_SOURCE_SAVED_OBJECT_TYPE, + 'data-source-id', + ['workspace-1'], + {} + ); + expect(clientMock.addToWorkspaces).toHaveBeenCalledWith( + DATA_SOURCE_SAVED_OBJECT_TYPE, + 'data-source-id', + ['workspace-1'], + {} + ); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + }); + it('should bypass permission check for call client.deleteFromWorkspaces', async () => { + await wrapper.deleteFromWorkspaces( + DATA_SOURCE_SAVED_OBJECT_TYPE, + 'data-source-id', + ['workspace-1'], + {} + ); + expect(clientMock.deleteFromWorkspaces).toHaveBeenCalledWith( + DATA_SOURCE_SAVED_OBJECT_TYPE, + 'data-source-id', + ['workspace-1'], + {} + ); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + }); + }); + + describe('addToWorkspaces', () => { + it('should throw error when non dashboard admin add data source to workspaces', async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(); + + let errorCatch; + try { + await wrapper.addToWorkspaces(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id', [ + 'workspace-id', + ]); + } catch (e) { + errorCatch = e; + } + expect(errorCatch.message).toEqual('Invalid permission, please contact OSD admin'); + }); + }); + + describe('deleteFromWorkspaces', () => { + it('should throw error when non dashboard admin delete data source from workspaces', async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(); + + let errorCatch; + try { + await wrapper.deleteFromWorkspaces(DATA_SOURCE_SAVED_OBJECT_TYPE, 'data-source-id', [ + 'workspace-id', + ]); + } catch (e) { + errorCatch = e; + } + expect(errorCatch.message).toEqual('Invalid permission, please contact OSD admin'); + }); }); }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index c5025da8c9b9..1c366f22fff9 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -415,6 +415,9 @@ export class WorkspaceSavedObjectsClientWrapper { const objectToGet = await wrapperOptions.client.get(type, id, options); if (objectToGet.type === DATA_SOURCE_SAVED_OBJECT_TYPE) { + if (isDataSourceAdmin) { + return objectToGet; + } const hasPermission = this.validateDataSourcePermissions( objectToGet, wrapperOptions.request @@ -475,6 +478,16 @@ export class WorkspaceSavedObjectsClientWrapper { const findWithWorkspacePermissionControl = async ( options: SavedObjectsFindOptions ) => { + if ( + isDataSourceAdmin && + options?.type && + (options.type === DATA_SOURCE_SAVED_OBJECT_TYPE || + (Array.isArray(options.type) && + options.type.length === 1 && + options.type[0] === DATA_SOURCE_SAVED_OBJECT_TYPE)) + ) { + return await wrapperOptions.client.find(options); + } const principals = this.permissionControl.getPrincipalsFromRequest(wrapperOptions.request); const permittedWorkspaceIds = ( await this.getWorkspaceTypeEnabledClient(wrapperOptions.request).find({ @@ -535,7 +548,35 @@ export class WorkspaceSavedObjectsClientWrapper { return await wrapperOptions.client.deleteByWorkspace(workspace, options); }; - const isDashboardAdmin = getWorkspaceState(wrapperOptions.request)?.isDashboardAdmin; + const addToWorkspacesWithPermissionControl = async ( + type: string, + id: string, + targetWorkspaces: string[], + options: SavedObjectsBaseOptions = {} + ) => { + // Only dashboard admin can assign data source to workspace + if (type === DATA_SOURCE_SAVED_OBJECT_TYPE) { + throw generateOSDAdminPermissionError(); + } + // In current version, only the type is data-source that will call addToWorkspaces + return await wrapperOptions.client.addToWorkspaces(type, id, targetWorkspaces, options); + }; + + const deleteFromWorkspacesWithPermissionControl = async ( + type: string, + id: string, + targetWorkspaces: string[], + options: SavedObjectsBaseOptions = {} + ) => { + // Only dashboard admin can unassign data source to workspace + if (type === DATA_SOURCE_SAVED_OBJECT_TYPE) { + throw generateOSDAdminPermissionError(); + } + // In current version, only the type is data-source will that call deleteFromWorkspaces + return await wrapperOptions.client.deleteFromWorkspaces(type, id, targetWorkspaces, options); + }; + + const { isDashboardAdmin, isDataSourceAdmin } = getWorkspaceState(wrapperOptions.request) || {}; if (isDashboardAdmin) { return wrapperOptions.client; } @@ -555,6 +596,8 @@ export class WorkspaceSavedObjectsClientWrapper { update: updateWithWorkspacePermissionControl, bulkUpdate: bulkUpdateWithWorkspacePermissionControl, deleteByWorkspace: deleteByWorkspaceWithPermissionControl, + addToWorkspaces: addToWorkspacesWithPermissionControl, + deleteFromWorkspaces: deleteFromWorkspacesWithPermissionControl, }; }; diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index b21f31e3accd..a52ab42183fb 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -135,11 +135,6 @@ export type IResponse = error?: string; }; -export interface AuthInfo { - backend_roles?: string[]; - user_name?: string; -} - export interface WorkspacePluginSetup { client: IWorkspaceClientImpl; } diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts index ba7532c216eb..f57c92fb4cdc 100644 --- a/src/plugins/workspace/server/utils.test.ts +++ b/src/plugins/workspace/server/utils.test.ts @@ -13,7 +13,6 @@ import { import { generateRandomId, getOSDAdminConfigFromYMLConfig, - getPrincipalsFromRequest, updateDashboardAdminStateForRequest, transferCurrentUserInPermissions, getDataSourcesList, @@ -24,7 +23,6 @@ import { Observable, of } from 'rxjs'; import { DEFAULT_DATA_SOURCE_UI_SETTINGS_ID } from '../../data_source_management/common'; describe('workspace utils', () => { - const mockAuth = httpServiceMock.createAuth(); it('should generate id with the specified size', () => { expect(generateRandomId(6)).toHaveLength(6); }); @@ -38,58 +36,6 @@ describe('workspace utils', () => { expect(ids.size).toBe(NUM_OF_ID); }); - it('should return empty map when request do not have authentication', () => { - const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); - mockAuth.get.mockReturnValueOnce({ - status: AuthStatus.unknown, - state: { - authInfo: { - user_name: 'bar', - backend_roles: ['foo'], - }, - }, - }); - const result = getPrincipalsFromRequest(mockRequest, mockAuth); - expect(result).toEqual({}); - }); - - it('should return normally when request has authentication', () => { - const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); - mockAuth.get.mockReturnValueOnce({ - status: AuthStatus.authenticated, - state: { - authInfo: { - user_name: 'bar', - backend_roles: ['foo'], - }, - }, - }); - const result = getPrincipalsFromRequest(mockRequest, mockAuth); - expect(result.users).toEqual(['bar']); - expect(result.groups).toEqual(['foo']); - }); - - it('should throw error when request is not authenticated', () => { - const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); - mockAuth.get.mockReturnValueOnce({ - status: AuthStatus.unauthenticated, - state: {}, - }); - expect(() => getPrincipalsFromRequest(mockRequest, mockAuth)).toThrow('NOT_AUTHORIZED'); - }); - - it('should throw error when authentication status is not expected', () => { - const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); - mockAuth.get.mockReturnValueOnce({ - // @ts-ignore - status: 'foo', - state: {}, - }); - expect(() => getPrincipalsFromRequest(mockRequest, mockAuth)).toThrow( - 'UNEXPECTED_AUTHORIZATION_STATUS' - ); - }); - it('should be dashboard admin when users match configUsers', () => { const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); const groups: string[] = ['dashboard_admin']; diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index 9f144c7eb1c3..b8c2b7613839 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -7,17 +7,12 @@ import crypto from 'crypto'; import { Observable } from 'rxjs'; import { first } from 'rxjs/operators'; import { - AuthStatus, - HttpAuth, OpenSearchDashboardsRequest, - Principals, - PrincipalType, SharedGlobalConfig, Permissions, SavedObjectsClientContract, IUiSettingsClient, } from '../../../core/server'; -import { AuthInfo } from './types'; import { updateWorkspaceState } from '../../../core/server/utils'; import { DEFAULT_DATA_SOURCE_UI_SETTINGS_ID } from '../../data_source_management/common'; import { CURRENT_USER_PLACEHOLDER } from '../common/constants'; @@ -29,37 +24,6 @@ export const generateRandomId = (size: number) => { return crypto.randomBytes(size).toString('base64url').slice(0, size); }; -export const getPrincipalsFromRequest = ( - request: OpenSearchDashboardsRequest, - auth?: HttpAuth -): Principals => { - const payload: Principals = {}; - const authInfoResp = auth?.get(request); - if (authInfoResp?.status === AuthStatus.unknown) { - /** - * Login user have access to all the workspaces when no authentication is presented. - */ - return payload; - } - - if (authInfoResp?.status === AuthStatus.authenticated) { - const authInfo = authInfoResp?.state as { authInfo: AuthInfo } | null; - if (authInfo?.authInfo?.backend_roles) { - payload[PrincipalType.Groups] = authInfo.authInfo.backend_roles; - } - if (authInfo?.authInfo?.user_name) { - payload[PrincipalType.Users] = [authInfo.authInfo.user_name]; - } - return payload; - } - - if (authInfoResp?.status === AuthStatus.unauthenticated) { - throw new Error('NOT_AUTHORIZED'); - } - - throw new Error('UNEXPECTED_AUTHORIZATION_STATUS'); -}; - export const updateDashboardAdminStateForRequest = ( request: OpenSearchDashboardsRequest, groups: string[], From 16d160a6fbd731fa5c829943e2697922ac2ebf5e Mon Sep 17 00:00:00 2001 From: Jincheng Wan <45655760+Kapian1234@users.noreply.github.com> Date: Thu, 5 Sep 2024 23:58:34 +0800 Subject: [PATCH 08/12] [Workspace] Refactor 'Associate data sources' in create page to support direct query connections (#7961) * support DQC Signed-off-by: Kapian1234 * Fix UTs in workspace form select data source panel Signed-off-by: Lin Wang * Remove no need IntlProvider Signed-off-by: Lin Wang * Add aria-labelledby for permission inputs Signed-off-by: Lin Wang * Modify UTs Signed-off-by: Kapian1234 * Changeset file for PR #7961 created/updated * Modify UTs Signed-off-by: Kapian1234 * Resolve some issues Signed-off-by: Kapian1234 * Modify UTs Signed-off-by: Kapian1234 * Fix UT errror Signed-off-by: Lin Wang * update button text Signed-off-by: Kapian1234 * rename onSelectItems() Signed-off-by: Kapian1234 * Fix an error Signed-off-by: Kapian1234 * Refactor data source connection table Signed-off-by: Lin Wang * resolve some issues Signed-off-by: Kapian1234 * resolve some issues Signed-off-by: Kapian1234 * Fix the data source URL reference Signed-off-by: Kapian1234 * Move restProps to tableProps Signed-off-by: Lin Wang * Fix table not unmont after connection type changed Signed-off-by: Lin Wang * Refactor selectedDataSources to selectedDataSourceConnections Signed-off-by: Lin Wang * Load direct query connections after data source tab selected Signed-off-by: Lin Wang --------- Signed-off-by: Kapian1234 Signed-off-by: Lin Wang Co-authored-by: Lin Wang Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/7961.yml | 2 + .../workspace_creator.test.tsx | 59 ++++- .../workspace_creator/workspace_creator.tsx | 14 +- .../workspace_creator_form.tsx | 10 +- .../workspace_form_summary_panel.test.tsx | 10 +- .../workspace_form_summary_panel.tsx | 4 +- .../select_data_source_panel.test.tsx | 80 +++--- .../select_data_source_panel.tsx | 126 +++++---- .../workspace_detail.test.tsx | 46 +++- .../workspace_detail/workspace_detail.tsx | 39 ++- ...orkspace_detail_connection_table.test.tsx} | 34 ++- .../workspace_detail_connection_table.tsx | 129 +++++++++ .../components/workspace_detail_app.tsx | 32 ++- .../components/workspace_form/constants.ts | 4 + .../data_source_connection_table.scss | 0 .../data_source_connection_table.tsx | 156 +++-------- .../public/components/workspace_form/index.ts | 2 + .../select_data_source_panel.test.tsx | 227 +++++++++++++--- .../select_data_source_panel.tsx | 244 +++++++++--------- .../public/components/workspace_form/types.ts | 10 +- .../workspace_form/use_workspace_form.ts | 17 +- .../components/workspace_form/utils.test.ts | 99 ++++++- .../public/components/workspace_form/utils.ts | 60 +++-- .../workspace_form/workspace_form_context.tsx | 13 +- .../workspace_permission_setting_input.tsx | 6 + .../workspace_permission_setting_panel.tsx | 37 ++- src/plugins/workspace/public/utils.ts | 69 +++-- 27 files changed, 1022 insertions(+), 507 deletions(-) create mode 100644 changelogs/fragments/7961.yml rename src/plugins/workspace/public/components/workspace_detail/{data_source_connection_table.test.tsx => workspace_detail_connection_table.test.tsx} (87%) create mode 100644 src/plugins/workspace/public/components/workspace_detail/workspace_detail_connection_table.tsx rename src/plugins/workspace/public/components/{workspace_detail => workspace_form}/data_source_connection_table.scss (100%) rename src/plugins/workspace/public/components/{workspace_detail => workspace_form}/data_source_connection_table.tsx (63%) diff --git a/changelogs/fragments/7961.yml b/changelogs/fragments/7961.yml new file mode 100644 index 000000000000..bdc020962e51 --- /dev/null +++ b/changelogs/fragments/7961.yml @@ -0,0 +1,2 @@ +feat: +- Support DQCs in create page ([#7961](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7961)) \ No newline at end of file diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx index 5ca958ced23e..4efbc30680ce 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx @@ -15,6 +15,9 @@ import { WorkspaceCreator as WorkspaceCreatorComponent, WorkspaceCreatorProps, } from './workspace_creator'; +import { DataSourceEngineType } from '../../../../data_source/common/data_sources'; +import { DataSourceConnectionType } from '../../../common/types'; +import * as utils from '../../utils'; const workspaceClientCreate = jest .fn() @@ -32,21 +35,50 @@ const dataSourcesList = [ { id: 'id1', title: 'ds1', + description: 'Description of data source 1', + auth: '', + dataSourceEngineType: '' as DataSourceEngineType, + workspaces: [], // This is used for mocking saved object function get: () => { return 'ds1'; }, }, { - id: '2', + id: 'id2', title: 'ds2', + description: 'Description of data source 1', + auth: '', + dataSourceEngineType: '' as DataSourceEngineType, + workspaces: [], get: () => { return 'ds2'; }, }, ]; +const dataSourceConnectionsList = [ + { + id: 'id1', + name: 'ds1', + connectionType: DataSourceConnectionType.OpenSearchConnection, + type: 'OpenSearch', + relatedConnections: [], + }, + { + id: 'id2', + name: 'ds2', + connectionType: DataSourceConnectionType.OpenSearchConnection, + type: 'OpenSearch', + }, +]; + const mockCoreStart = coreMock.createStart(); +jest.spyOn(utils, 'fetchDataSourceConnections').mockImplementation(async (passedDataSources) => { + return dataSourceConnectionsList.filter(({ id }) => + passedDataSources.some((dataSource) => dataSource.id === id) + ); +}); const WorkspaceCreator = ({ isDashboardAdmin = false, @@ -304,7 +336,15 @@ describe('WorkspaceCreator', () => { }); it('create workspace with customized selected dataSources', async () => { - const { getByTestId, getByTitle, getByText } = render( + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { + configurable: true, + value: 600, + }); + Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { + configurable: true, + value: 600, + }); + const { getByTestId, getAllByText, getByText } = render( ); @@ -317,10 +357,17 @@ describe('WorkspaceCreator', () => { target: { value: 'test workspace name' }, }); fireEvent.click(getByTestId('workspaceUseCase-observability')); - fireEvent.click(getByTestId('workspaceForm-select-dataSource-addNew')); - fireEvent.click(getByTestId('workspaceForm-select-dataSource-comboBox')); - fireEvent.click(getByText('Select')); - fireEvent.click(getByTitle(dataSourcesList[0].title)); + fireEvent.click(getByTestId('workspace-creator-dataSources-assign-button')); + await waitFor(() => { + expect( + getByText( + 'Add data sources that will be available in the workspace. If a selected data source has related Direct Query connection, they will also be available in the workspace.' + ) + ).toBeInTheDocument(); + expect(getByText(dataSourcesList[0].title)).toBeInTheDocument(); + }); + fireEvent.click(getByText(dataSourcesList[0].title)); + fireEvent.click(getAllByText('Associate data sources')[1]); fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); expect(workspaceClientCreate).toHaveBeenCalledWith( diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx index b8f32eb4a33e..ed4370a7b3f5 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -15,12 +15,12 @@ import { getUseCaseFeatureConfig } from '../../../common/utils'; import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; import { WorkspaceClient } from '../../workspace_client'; import { convertPermissionSettingsToPermissions } from '../workspace_form'; -import { DataSource } from '../../../common/types'; import { DataSourceManagementPluginSetup } from '../../../../../plugins/data_source_management/public'; import { WorkspaceUseCase } from '../../types'; import { getFirstUseCaseOfFeatureConfigs } from '../../utils'; import { useFormAvailableUseCases } from '../workspace_form/use_form_available_use_cases'; import { NavigationPublicPluginStart } from '../../../../../plugins/navigation/public'; +import { DataSourceConnectionType } from '../../../common/types'; import { WorkspaceCreatorForm } from './workspace_creator_form'; export interface WorkspaceCreatorProps { @@ -72,10 +72,14 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { } setIsFormSubmitting(true); try { - const { permissionSettings, selectedDataSources, ...attributes } = data; - const selectedDataSourceIds = (selectedDataSources ?? []).map((ds: DataSource) => { - return ds.id; - }); + const { permissionSettings, selectedDataSourceConnections, ...attributes } = data; + const selectedDataSourceIds = (selectedDataSourceConnections ?? []) + .filter( + ({ connectionType }) => connectionType === DataSourceConnectionType.OpenSearchConnection + ) + .map(({ id }) => { + return id; + }); result = await workspaceClient.create(attributes, { dataSources: selectedDataSourceIds, permissions: convertPermissionSettingsToPermissions(permissionSettings), diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator_form.tsx index 9ab0a35e722b..4a99fc006524 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator_form.tsx @@ -58,7 +58,7 @@ export const WorkspaceCreatorForm = (props: WorkspaceCreatorFormProps) => { handleColorChange, handleUseCaseChange: handleUseCaseChangeInHook, setPermissionSettings, - setSelectedDataSources, + setSelectedDataSourceConnections, } = useWorkspaceForm(props); const nameManualChangedRef = useRef(false); @@ -86,7 +86,7 @@ export const WorkspaceCreatorForm = (props: WorkspaceCreatorFormProps) => { return ( - + { })} diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_form_summary_panel.test.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_form_summary_panel.test.tsx index 9ce350af85e2..cfe9e5833631 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_form_summary_panel.test.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_form_summary_panel.test.tsx @@ -20,10 +20,10 @@ describe('WorkspaceFormSummaryPanel', () => { name: 'Test Workspace', description: 'This is a test workspace', color: '#000000', - selectedDataSources: [ - { id: 'data-source-1', title: 'Data Source 1' }, - { id: 'data-source-2', title: 'Data Source 2' }, - { id: 'data-source-3', title: 'Data Source 3' }, + selectedDataSourceConnections: [ + { id: 'data-source-1', name: 'Data Source 1' }, + { id: 'data-source-2', name: 'Data Source 2' }, + { id: 'data-source-3', name: 'Data Source 3' }, ], permissionSettings: [ { id: 1, type: WorkspacePermissionItemType.User, userId: 'user1' }, @@ -74,7 +74,7 @@ describe('WorkspaceFormSummaryPanel', () => { - {formData.selectedDataSources.length > 0 && ( + {formData.selectedDataSourceConnections.length > 0 && ( title)} + texts={formData.selectedDataSourceConnections.map(({ name }) => name)} collapseDisplayCount={2} /> )} diff --git a/src/plugins/workspace/public/components/workspace_detail/select_data_source_panel.test.tsx b/src/plugins/workspace/public/components/workspace_detail/select_data_source_panel.test.tsx index 1329bef04beb..1c15eac21193 100644 --- a/src/plugins/workspace/public/components/workspace_detail/select_data_source_panel.test.tsx +++ b/src/plugins/workspace/public/components/workspace_detail/select_data_source_panel.test.tsx @@ -8,10 +8,13 @@ import React from 'react'; import { coreMock } from '../../../../../core/public/mocks'; import { createOpenSearchDashboardsReactContext } from '../../../../opensearch_dashboards_react/public'; import { WorkspaceFormProvider, WorkspaceOperationType } from '../workspace_form'; -import { SelectDataSourceDetailPanel } from './select_data_source_panel'; +import { + SelectDataSourceDetailPanel, + SelectDataSourceDetailPanelProps, +} from './select_data_source_panel'; import * as utils from '../../utils'; import { IntlProvider } from 'react-intl'; -import { DataSourceConnectionType } from '../../../common/types'; +import { DataSourceConnection, DataSourceConnectionType } from '../../../common/types'; const mockCoreStart = coreMock.createStart(); @@ -81,8 +84,7 @@ const defaultValues = { }; const defaultProps = { - savedObjects: {}, - assignedDataSources: [], + savedObjects: mockCoreStart.savedObjects, detailTitle: 'Data sources', isDashboardAdmin: true, currentWorkspace: workspaceObject, @@ -96,7 +98,14 @@ const success = jest.fn().mockResolvedValue({ }); const failed = jest.fn().mockResolvedValue({}); -const selectDataSourceDetailPanel = (props: any) => { +const selectDataSourceDetailPanel = ({ + action, + selectedDataSourceConnections, + ...props +}: { + action?: Function; + selectedDataSourceConnections?: DataSourceConnection[]; +} & Partial) => { const { Provider } = createOpenSearchDashboardsReactContext({ ...mockCoreStart, ...{ @@ -109,7 +118,7 @@ const selectDataSourceDetailPanel = (props: any) => { }, }, workspaceClient: { - update: props.action, + update: action, }, }, }); @@ -122,11 +131,16 @@ const selectDataSourceDetailPanel = (props: any) => { operationType={WorkspaceOperationType.Update} permissionEnabled={true} onSubmit={jest.fn()} - defaultValues={defaultValues} + defaultValues={{ ...defaultValues, selectedDataSourceConnections }} availableUseCases={[]} > - + @@ -134,12 +148,11 @@ const selectDataSourceDetailPanel = (props: any) => { }; describe('SelectDataSourceDetailPanel', () => { - afterEach(() => { + beforeEach(() => { jest.clearAllMocks(); }); it('should show message when no data sources are assigned', async () => { - jest.spyOn(utils, 'fetchDataSourceConnections').mockResolvedValue([]); const { getByText, getAllByText } = render(selectDataSourceDetailPanel(defaultProps)); await waitFor(() => { expect(getByText('No data sources to display')).toBeInTheDocument(); @@ -151,7 +164,6 @@ describe('SelectDataSourceDetailPanel', () => { }); it('should not show assocition button when user is not OSD admin', async () => { - jest.spyOn(utils, 'fetchDataSourceConnections').mockResolvedValue([]); const { getByText, queryByText } = render( selectDataSourceDetailPanel({ ...defaultProps, @@ -167,11 +179,9 @@ describe('SelectDataSourceDetailPanel', () => { }); it('should not show remove associations button when user is not OSD admin', async () => { - jest.spyOn(utils, 'fetchDataSourceConnections').mockResolvedValue(dataSourceConnectionsMock); const { queryByTestId } = render( selectDataSourceDetailPanel({ ...defaultProps, - assignedDataSources: dataSources, isDashboardAdmin: false, }) ); @@ -181,7 +191,6 @@ describe('SelectDataSourceDetailPanel', () => { }); it('should switch toggle button', async () => { - jest.spyOn(utils, 'fetchDataSourceConnections').mockResolvedValue(dataSourceConnectionsMock); const { getByText } = render(selectDataSourceDetailPanel(defaultProps)); await waitFor(() => { const dqcButton = getByText('Direct query connections'); @@ -200,7 +209,6 @@ describe('SelectDataSourceDetailPanel', () => { value: 600, }); jest.spyOn(utils, 'getDataSourcesList').mockResolvedValue([]); - jest.spyOn(utils, 'fetchDataSourceConnections').mockResolvedValueOnce([]); jest .spyOn(utils, 'fetchDataSourceConnections') .mockResolvedValueOnce(dataSourceConnectionsMock); @@ -245,7 +253,6 @@ describe('SelectDataSourceDetailPanel', () => { value: 600, }); jest.spyOn(utils, 'getDataSourcesList').mockResolvedValue([]); - jest.spyOn(utils, 'fetchDataSourceConnections').mockResolvedValueOnce([]); jest .spyOn(utils, 'fetchDataSourceConnections') .mockResolvedValueOnce(dataSourceConnectionsMock); @@ -317,13 +324,10 @@ describe('SelectDataSourceDetailPanel', () => { }); it('should success to remove data sources', async () => { - jest - .spyOn(utils, 'fetchDataSourceConnections') - .mockResolvedValueOnce([dataSourceConnectionsMock[0]]); const { getByText, getByTestId, getByRole } = render( selectDataSourceDetailPanel({ ...defaultProps, - assignedDataSources: [dataSources[0]], + selectedDataSourceConnections: [dataSourceConnectionsMock[0]], action: success, }) ); @@ -341,13 +345,10 @@ describe('SelectDataSourceDetailPanel', () => { }); it('should fail to remove data sources', async () => { - jest - .spyOn(utils, 'fetchDataSourceConnections') - .mockResolvedValueOnce([dataSourceConnectionsMock[0]]); const { getByText, getByTestId, getByRole } = render( selectDataSourceDetailPanel({ ...defaultProps, - assignedDataSources: [dataSources[0]], + selectedDataSourceConnections: [dataSourceConnectionsMock[0]], action: failed, }) ); @@ -365,13 +366,10 @@ describe('SelectDataSourceDetailPanel', () => { }); it('should remove selected data sources successfully', async () => { - jest - .spyOn(utils, 'fetchDataSourceConnections') - .mockResolvedValueOnce([dataSourceConnectionsMock[0]]); const { getByText, queryByTestId, getAllByRole, getByRole } = render( selectDataSourceDetailPanel({ ...defaultProps, - assignedDataSources: [dataSources[0]], + selectedDataSourceConnections: [dataSourceConnectionsMock[0]], action: success, }) ); @@ -392,22 +390,38 @@ describe('SelectDataSourceDetailPanel', () => { }); it('should handle input in the search box', async () => { - jest.spyOn(utils, 'fetchDataSourceConnections').mockResolvedValue(dataSourceConnectionsMock); const { getByText, queryByText } = render( selectDataSourceDetailPanel({ ...defaultProps, - assignedDataSources: dataSources, + selectedDataSourceConnections: dataSourceConnectionsMock, }) ); await waitFor(() => { expect(getByText('Data Source 1')).toBeInTheDocument(); expect(getByText('Data Source 2')).toBeInTheDocument(); + }); + + const searchInput = screen.getByPlaceholderText('Search...'); + // Simulate typing in the search input + fireEvent.change(searchInput, { target: { value: 'Data Source 1' } }); - const searchInput = screen.getByPlaceholderText('Search...'); - // Simulate typing in the search input - fireEvent.change(searchInput, { target: { value: 'Data Source 1' } }); + await waitFor(() => { expect(getByText('Data Source 1')).toBeInTheDocument(); expect(queryByText('Data Source 2')).toBeNull(); }); }); + + it('should show loading message when loading', async () => { + const { queryByText, getByText, rerender } = render( + selectDataSourceDetailPanel({ loading: false }) + ); + await waitFor(() => { + expect(queryByText('Loading data sources...')).not.toBeInTheDocument(); + }); + + rerender(selectDataSourceDetailPanel({ loading: true })); + await waitFor(() => { + expect(getByText('Loading data sources...')).toBeInTheDocument(); + }); + }); }); diff --git a/src/plugins/workspace/public/components/workspace_detail/select_data_source_panel.tsx b/src/plugins/workspace/public/components/workspace_detail/select_data_source_panel.tsx index 1f13da230f9e..7a503265983d 100644 --- a/src/plugins/workspace/public/components/workspace_detail/select_data_source_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_detail/select_data_source_panel.tsx @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useState } from 'react'; import { EuiText, EuiTitle, @@ -21,9 +21,10 @@ import { } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { FormattedMessage } from 'react-intl'; -import { DataSource, DataSourceConnection, DataSourceConnectionType } from '../../../common/types'; +import { useUpdateEffect } from 'react-use'; +import { DataSourceConnection, DataSourceConnectionType } from '../../../common/types'; import { WorkspaceClient } from '../../workspace_client'; -import { DataSourceConnectionTable } from './data_source_connection_table'; +import { WorkspaceDetailConnectionTable } from './workspace_detail_connection_table'; import { AssociationDataSourceModal } from './association_data_source_modal'; import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; import { @@ -32,8 +33,11 @@ import { WorkspaceObject, ChromeStart, } from '../../../../../core/public'; -import { convertPermissionSettingsToPermissions, useWorkspaceFormContext } from '../workspace_form'; -import { fetchDataSourceConnections } from '../../utils'; +import { + convertPermissionSettingsToPermissions, + isWorkspacePermissionSetting, + useWorkspaceFormContext, +} from '../workspace_form'; import { AssociationDataSourceModalMode } from '../../../common/constants'; const toggleButtons: EuiButtonGroupOptionProps[] = [ @@ -50,66 +54,59 @@ const toggleButtons: EuiButtonGroupOptionProps[] = [ }), }, ]; -export interface SelectDataSourcePanelProps { +export interface SelectDataSourceDetailPanelProps { savedObjects: SavedObjectsStart; - assignedDataSources: DataSource[]; detailTitle: string; isDashboardAdmin: boolean; currentWorkspace: WorkspaceObject; chrome: ChromeStart; + loading?: boolean; } export const SelectDataSourceDetailPanel = ({ - assignedDataSources, savedObjects, detailTitle, isDashboardAdmin, currentWorkspace, chrome, -}: SelectDataSourcePanelProps) => { + loading = false, +}: SelectDataSourceDetailPanelProps) => { const { services: { notifications, workspaceClient, http }, } = useOpenSearchDashboards<{ CoreStart: CoreStart; workspaceClient: WorkspaceClient }>(); - const { formData, setSelectedDataSources } = useWorkspaceFormContext(); - const [isLoading, setIsLoading] = useState(false); + const { formData, setSelectedDataSourceConnections } = useWorkspaceFormContext(); + const [isLoading, setIsLoading] = useState(loading); const [isVisible, setIsVisible] = useState(false); - const [assignedDataSourceConnections, setAssignedDataSourceConnections] = useState< - DataSourceConnection[] - >([]); const [toggleIdSelected, setToggleIdSelected] = useState(toggleButtons[0].id); - useEffect(() => { - setIsLoading(true); - fetchDataSourceConnections(assignedDataSources, http, notifications).then((connections) => { - setAssignedDataSourceConnections(connections); - setIsLoading(false); - }); - }, [assignedDataSources, http, notifications]); - const handleAssignDataSourceConnections = async ( - dataSourceConnections: DataSourceConnection[] + newAssignedDataSourceConnections: DataSourceConnection[] ) => { - const dataSources = dataSourceConnections - .filter( - ({ connectionType }) => connectionType === DataSourceConnectionType.OpenSearchConnection - ) - .map(({ id, type, name, description }) => ({ - id, - title: name, - description, - dataSourceEngineType: type, - })); try { setIsLoading(true); setIsVisible(false); - const { permissionSettings, selectedDataSources, useCase, ...attributes } = formData; - const savedDataSources: DataSource[] = [...selectedDataSources, ...dataSources]; + const { + permissionSettings, + selectedDataSourceConnections, + useCase, + ...attributes + } = formData; + + const savedDataSourceConnections = [ + ...formData.selectedDataSourceConnections, + ...newAssignedDataSourceConnections, + ]; const result = await workspaceClient.update(currentWorkspace.id, attributes, { - dataSources: savedDataSources.map((ds) => { - return ds.id; - }), - permissions: convertPermissionSettingsToPermissions(permissionSettings), + dataSources: savedDataSourceConnections + .filter( + ({ connectionType }) => connectionType === DataSourceConnectionType.OpenSearchConnection + ) + .map(({ id }) => id), + // Todo: Make permissions be an optional parameter when update workspace + permissions: convertPermissionSettingsToPermissions( + permissionSettings.filter(isWorkspacePermissionSetting) + ), }); if (result?.success) { notifications?.toasts.addSuccess({ @@ -117,7 +114,7 @@ export const SelectDataSourceDetailPanel = ({ defaultMessage: 'Associate OpenSearch connections successfully', }), }); - setSelectedDataSources(savedDataSources); + setSelectedDataSourceConnections(savedDataSourceConnections); } else { throw new Error(result?.error ? result?.error : 'Associate OpenSearch connections failed'); } @@ -134,17 +131,30 @@ export const SelectDataSourceDetailPanel = ({ }; const handleUnassignDataSources = useCallback( - async (dataSources: DataSourceConnection[]) => { + async (unAssignedDataSources: DataSourceConnection[]) => { try { setIsLoading(true); - const { permissionSettings, selectedDataSources, useCase, ...attributes } = formData; - const savedDataSources = (selectedDataSources ?? [])?.filter( - ({ id }: DataSource) => !dataSources.some((item) => item.id === id) + const { + permissionSettings, + selectedDataSourceConnections, + useCase, + ...attributes + } = formData; + const savedDataSourceConnections = selectedDataSourceConnections.filter( + ({ id }) => !unAssignedDataSources.some((item) => item.id === id) ); const result = await workspaceClient.update(currentWorkspace.id, attributes, { - dataSources: savedDataSources.map(({ id }: DataSource) => id), - permissions: convertPermissionSettingsToPermissions(permissionSettings), + dataSources: savedDataSourceConnections + .filter( + ({ connectionType }) => + connectionType === DataSourceConnectionType.OpenSearchConnection + ) + .map(({ id }) => id), + // Todo: Make permissions be an optional parameter when update workspace + permissions: convertPermissionSettingsToPermissions( + permissionSettings.filter(isWorkspacePermissionSetting) + ), }); if (result?.success) { notifications?.toasts.addSuccess({ @@ -152,7 +162,7 @@ export const SelectDataSourceDetailPanel = ({ defaultMessage: 'Remove associated OpenSearch connections successfully', }), }); - setSelectedDataSources(savedDataSources); + setSelectedDataSourceConnections(savedDataSourceConnections); } else { throw new Error( result?.error ? result?.error : 'Remove associated OpenSearch connections failed' @@ -169,7 +179,13 @@ export const SelectDataSourceDetailPanel = ({ setIsLoading(false); } }, - [currentWorkspace.id, formData, notifications?.toasts, setSelectedDataSources, workspaceClient] + [ + currentWorkspace.id, + formData, + notifications?.toasts, + setSelectedDataSourceConnections, + workspaceClient, + ] ); const associationButton = ( @@ -194,7 +210,7 @@ export const SelectDataSourceDetailPanel = ({ @@ -240,19 +256,23 @@ export const SelectDataSourceDetailPanel = ({ if (isLoading) { return loadingMessage; } - if (assignedDataSources.length === 0) { + if (formData.selectedDataSourceConnections.length === 0) { return noAssociationMessage; } return ( - ); }; + useUpdateEffect(() => { + setIsLoading(loading); + }, [loading]); + return ( @@ -284,7 +304,7 @@ export const SelectDataSourceDetailPanel = ({ notifications={notifications} savedObjects={savedObjects} closeModal={() => setIsVisible(false)} - assignedConnections={assignedDataSourceConnections} + assignedConnections={formData.selectedDataSourceConnections} handleAssignDataSourceConnections={handleAssignDataSourceConnections} mode={toggleIdSelected as AssociationDataSourceModalMode} logos={chrome.logos} diff --git a/src/plugins/workspace/public/components/workspace_detail/workspace_detail.test.tsx b/src/plugins/workspace/public/components/workspace_detail/workspace_detail.test.tsx index 6004e4de206b..dffc12da3bbc 100644 --- a/src/plugins/workspace/public/components/workspace_detail/workspace_detail.test.tsx +++ b/src/plugins/workspace/public/components/workspace_detail/workspace_detail.test.tsx @@ -3,16 +3,18 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { fireEvent, render, screen } from '@testing-library/react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import React from 'react'; import { BehaviorSubject } from 'rxjs'; +import { MemoryRouter } from 'react-router-dom'; import { PublicAppInfo, WorkspaceObject } from 'opensearch-dashboards/public'; import { coreMock } from '../../../../../core/public/mocks'; import { createOpenSearchDashboardsReactContext } from '../../../../opensearch_dashboards_react/public'; import { createMockedRegisteredUseCases$ } from '../../mocks'; import { WorkspaceDetail } from './workspace_detail'; import { WorkspaceFormProvider, WorkspaceOperationType } from '../workspace_form'; -import { MemoryRouter } from 'react-router-dom'; +import { DataSourceConnectionType } from '../../../common/types'; +import * as utilsExports from '../../utils'; // all applications const PublicAPPInfoMap = new Map([ @@ -52,12 +54,13 @@ const defaultValues = { modes: ['library_write', 'write'], }, ], - selectedDataSources: [ + selectedDataSourceConnections: [ { id: 'ds-1', - title: 'ds-1-title', + name: 'ds-1-title', description: 'ds-1-description', - dataSourceEngineType: 'OpenSearch', + type: 'OpenSearch', + connectionType: DataSourceConnectionType.OpenSearchConnection, }, ], }; @@ -200,6 +203,9 @@ describe('WorkspaceDetail', () => { const { getByText } = render(WorkspaceDetailPage({ workspacesService: workspaceService })); fireEvent.click(getByText('Data sources')); expect(document.querySelector('#dataSources')).toHaveClass('euiTab-isSelected'); + await waitFor(() => { + expect(getByText('Loading data sources...')).toBeInTheDocument(); + }); }); it('delete button will been shown at page header', async () => { @@ -297,4 +303,34 @@ describe('WorkspaceDetail', () => { expect(alertSpy).toBeCalledTimes(0); alertSpy.mockRestore(); }); + + it('should show loaded data sources', async () => { + jest.spyOn(utilsExports, 'fetchDataSourceConnectionsByDataSourceIds').mockResolvedValue([ + { + id: 'dqc-1', + name: 'dqc-1-title', + description: 'dqc-1-description', + type: 'Amazon S3', + parentId: 'ds-1', + connectionType: DataSourceConnectionType.DirectQueryConnection, + }, + { + id: 'dqc-2', + name: 'dqc-1-title', + description: 'dqc-1-description', + type: 'Amazon S3', + parentId: 'ds-1', + connectionType: DataSourceConnectionType.DirectQueryConnection, + }, + ]); + const workspaceService = createWorkspacesSetupContractMockWithValue(workspaceObject); + const { getByText, getByRole } = render( + WorkspaceDetailPage({ workspacesService: workspaceService }) + ); + fireEvent.click(getByText('Data sources')); + await waitFor(() => { + expect(getByText('ds-1-title')).toBeInTheDocument(); + expect(getByRole('button', { name: '2' })).toBeInTheDocument(); + }); + }); }); diff --git a/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx b/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx index 3d4b77c20a22..052a54dcc785 100644 --- a/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx +++ b/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx @@ -24,7 +24,12 @@ import { WORKSPACE_LIST_APP_ID } from '../../../common/constants'; import { cleanWorkspaceId } from '../../../../../core/public/utils'; import { DetailTab, DetailTabTitles, WorkspaceOperationType } from '../workspace_form/constants'; import { CoreStart, WorkspaceAttribute } from '../../../../../core/public'; -import { getFirstUseCaseOfFeatureConfigs, getUseCaseUrl } from '../../utils'; +import { + fetchDataSourceConnectionsByDataSourceIds, + fulfillRelatedConnections, + getFirstUseCaseOfFeatureConfigs, + getUseCaseUrl, +} from '../../utils'; import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; import { DataSourceManagementPluginSetup } from '../../../../../plugins/data_source_management/public'; import { SelectDataSourceDetailPanel } from './select_data_source_panel'; @@ -50,6 +55,7 @@ export const WorkspaceDetail = (props: WorkspaceDetailProps) => { uiSettings, navigationUI: { HeaderControl }, chrome, + notifications, }, } = useOpenSearchDashboards<{ CoreStart: CoreStart; @@ -58,18 +64,20 @@ export const WorkspaceDetail = (props: WorkspaceDetailProps) => { }>(); const { - formData, isEditing, formId, numberOfErrors, handleResetForm, numberOfChanges, setIsEditing, + formData, + setSelectedDataSourceConnections, } = useWorkspaceFormContext(); const [deletedWorkspace, setDeletedWorkspace] = useState(null); const [selectedTabId, setSelectedTabId] = useState(DetailTab.Details); const [modalVisible, setModalVisible] = useState(false); const [tabId, setTabId] = useState(DetailTab.Details); + const [isDQCFilled, setIsDQCFilled] = useState(false); const availableUseCases = useObservable(props.registeredUseCases$, []); const isDashboardAdmin = !!application?.capabilities?.dashboards?.isDashboardAdmin; @@ -89,6 +97,31 @@ export const WorkspaceDetail = (props: WorkspaceDetailProps) => { } }, [location.search]); + useEffect(() => { + if (selectedTabId !== DetailTab.DataSources || isDQCFilled || !http || !notifications) { + return; + } + fetchDataSourceConnectionsByDataSourceIds( + formData.selectedDataSourceConnections.map(({ id }) => id), + http + ) + .then((directQueryConnections) => { + setSelectedDataSourceConnections( + fulfillRelatedConnections(formData.selectedDataSourceConnections, directQueryConnections) + ); + }) + .finally(() => { + setIsDQCFilled(true); + }); + }, [ + http, + isDQCFilled, + selectedTabId, + notifications, + setSelectedDataSourceConnections, + formData.selectedDataSourceConnections, + ]); + if (!currentWorkspace || !application || !http || !savedObjects || !uiSettings || !chrome) { return null; } @@ -141,8 +174,8 @@ export const WorkspaceDetail = (props: WorkspaceDetailProps) => { name: DetailTabTitles.dataSources, content: ( ({ + ...jest.requireActual('../../../../opensearch_dashboards_react/public'), + useOpenSearchDashboards: jest.fn(), +})); const handleUnassignDataSources = jest.fn(); const dataSourceConnectionsMock = [ { @@ -57,14 +62,27 @@ const dataSourceConnectionsMock = [ }, ]; -describe('DataSourceConnectionTable', () => { +describe('WorkspaceDetailConnectionTable', () => { + beforeEach(() => { + const mockPrepend = jest.fn().mockImplementation((path) => path); + const mockHttp = { + basePath: { + prepend: mockPrepend, + }, + }; + (useOpenSearchDashboards as jest.Mock).mockImplementation(() => ({ + services: { + http: mockHttp, + }, + })); + }); afterEach(() => { jest.clearAllMocks(); }); describe('OpenSearch connections', () => { it('renders the table with OpenSearch connections', () => { const { getByText, queryByText } = render( - { it('should show dqc popover when click the Related connections number ', () => { const { getByText } = render( - { it('should remove selected OpenSearch connections by dashboard admin', () => { const { getByText, queryByTestId, getAllByRole, getByRole } = render( - { it('should remove single OpenSearch connections by dashboard admin', () => { const { queryAllByTestId, getByText, getByRole } = render( - { it('should hide remove action iif user is not dashboard admin', () => { const { queryByText, queryByTestId, getAllByRole } = render( - { describe('Direct query connections', () => { it('renders the table with Direct query connections', () => { const { getByText, queryByText, getByTestId } = render( - void; +} + +export const WorkspaceDetailConnectionTable = ({ + isDashboardAdmin, + connectionType, + dataSourceConnections, + handleUnassignDataSources, +}: WorkspaceDetailConnectionTableProps) => { + const [selectedItems, setSelectedItems] = useState([]); + const [modalVisible, setModalVisible] = useState(false); + + useEffect(() => { + // Reset selected items when connectionType changes + setSelectedItems([]); + }, [connectionType]); + + const openSearchConnections = useMemo(() => { + return dataSourceConnections.filter((dsc) => + connectionType === AssociationDataSourceModalMode.OpenSearchConnections + ? dsc.connectionType === DataSourceConnectionType.OpenSearchConnection + : dsc?.relatedConnections && dsc.relatedConnections?.length > 0 + ); + }, [connectionType, dataSourceConnections]); + + const renderToolsLeft = useCallback(() => { + return selectedItems.length > 0 && !modalVisible + ? [ + setModalVisible(true)} + data-test-subj="workspace-detail-dataSources-table-bulkRemove" + > + {i18n.translate('workspace.detail.dataSources.table.remove.button', { + defaultMessage: 'Remove {numberOfSelect} association(s)', + values: { numberOfSelect: selectedItems.length }, + })} + , + ] + : []; + }, [selectedItems, modalVisible]); + + const search: EuiSearchBarProps = { + toolsLeft: renderToolsLeft(), + box: { + incremental: true, + }, + filters: [ + { + type: 'field_value_selection', + field: 'type', + name: 'Type', + multiSelect: 'or', + options: Array.from( + new Set(openSearchConnections.map(({ type }) => type).filter(Boolean)) + ).map((type) => ({ + value: type!, + name: type!, + })), + }, + ], + }; + + return ( + <> + { + { + setSelectedItems([item]); + setModalVisible(true); + }} + onSelectionChange={setSelectedItems} + tableProps={{ + search, + pagination: { + initialPageSize: 10, + pageSizeOptions: [10, 20, 30], + }, + }} + /* Unmount table after connection type */ + key={connectionType} + /> + } + {modalVisible && ( + { + setModalVisible(false); + setSelectedItems([]); + }} + onConfirm={() => { + setModalVisible(false); + handleUnassignDataSources(selectedItems); + }} + cancelButtonText={i18n.translate('workspace.detail.dataSources.modal.cancelButton', { + defaultMessage: 'Cancel', + })} + confirmButtonText={i18n.translate('workspace.detail.dataSources.Modal.confirmButton', { + defaultMessage: 'Remove data source(s)', + })} + buttonColor="danger" + defaultFocusedButton="confirm" + /> + )} + + ); +}; diff --git a/src/plugins/workspace/public/components/workspace_detail_app.tsx b/src/plugins/workspace/public/components/workspace_detail_app.tsx index 1347b130575b..9518b5707bb3 100644 --- a/src/plugins/workspace/public/components/workspace_detail_app.tsx +++ b/src/plugins/workspace/public/components/workspace_detail_app.tsx @@ -19,11 +19,11 @@ import { convertPermissionSettingsToPermissions, convertPermissionsToPermissionSettings, } from './workspace_form'; -import { DataSource } from '../../common/types'; +import { DataSourceConnectionType } from '../../common/types'; import { WorkspaceClient } from '../workspace_client'; import { formatUrlWithWorkspaceId } from '../../../../core/public/utils'; import { WORKSPACE_DETAIL_APP_ID } from '../../common/constants'; -import { getDataSourcesList } from '../utils'; +import { getDataSourcesList, mergeDataSourcesWithConnections } from '../utils'; import { WorkspaceAttributeWithPermission } from '../../../../core/types'; function getFormDataFromWorkspace( @@ -34,16 +34,13 @@ function getFormDataFromWorkspace( } return { ...currentWorkspace, + features: currentWorkspace.features ?? [], permissionSettings: currentWorkspace.permissions ? convertPermissionsToPermissionSettings(currentWorkspace.permissions) : currentWorkspace.permissions, }; } -type FormDataFromWorkspace = ReturnType & { - selectedDataSources: DataSource[]; -}; - export const WorkspaceDetailApp = (props: WorkspaceDetailProps) => { const { services: { @@ -56,7 +53,9 @@ export const WorkspaceDetailApp = (props: WorkspaceDetailProps) => { http, }, } = useOpenSearchDashboards<{ CoreStart: CoreStart; workspaceClient: WorkspaceClient }>(); - const [currentWorkspaceFormData, setCurrentWorkspaceFormData] = useState(); + const [currentWorkspaceFormData, setCurrentWorkspaceFormData] = useState< + WorkspaceFormSubmitData + >(); const currentWorkspace = useObservable(workspaces ? workspaces.currentWorkspace$ : of(null)); const availableUseCases = useObservable(props.registeredUseCases$, []); const isPermissionEnabled = application?.capabilities.workspaces.permissionEnabled; @@ -93,14 +92,15 @@ export const WorkspaceDetailApp = (props: WorkspaceDetailProps) => { const rawFormData = getFormDataFromWorkspace(currentWorkspace); if (rawFormData && savedObjects && currentWorkspace) { - getDataSourcesList(savedObjects.client, [currentWorkspace.id]).then((selectedDataSources) => { + getDataSourcesList(savedObjects.client, [currentWorkspace.id]).then((dataSources) => { setCurrentWorkspaceFormData({ ...rawFormData, - selectedDataSources, + // Direct query connections info is not required for all tabs, it can be fetched later + selectedDataSourceConnections: mergeDataSourcesWithConnections(dataSources, []), }); }); } - }, [currentWorkspace, savedObjects]); + }, [currentWorkspace, savedObjects, http, notifications]); const handleWorkspaceFormSubmit = useCallback( async (data: WorkspaceFormSubmitData) => { @@ -115,10 +115,14 @@ export const WorkspaceDetailApp = (props: WorkspaceDetailProps) => { } try { - const { permissionSettings, selectedDataSources, ...attributes } = data; - const selectedDataSourceIds = (selectedDataSources ?? []).map((ds: DataSource) => { - return ds.id; - }); + const { permissionSettings, selectedDataSourceConnections, ...attributes } = data; + const selectedDataSourceIds = (selectedDataSourceConnections ?? []) + .filter( + ({ connectionType }) => connectionType === DataSourceConnectionType.OpenSearchConnection + ) + .map((connection) => { + return connection.id; + }); result = await workspaceClient.update(currentWorkspace.id, attributes, { dataSources: selectedDataSourceIds, diff --git a/src/plugins/workspace/public/components/workspace_form/constants.ts b/src/plugins/workspace/public/components/workspace_form/constants.ts index ed2268bec8d7..c09ac11f5489 100644 --- a/src/plugins/workspace/public/components/workspace_form/constants.ts +++ b/src/plugins/workspace/public/components/workspace_form/constants.ts @@ -123,3 +123,7 @@ export const DetailTabTitles: { [key in DetailTab]: string } = { defaultMessage: 'Collaborators', }), }; + +export const PERMISSION_TYPE_LABEL_ID = 'workspace-form-permission-type-label'; +export const PERMISSION_COLLABORATOR_LABEL_ID = 'workspace-form-permission-collaborator-label'; +export const PERMISSION_ACCESS_LEVEL_LABEL_ID = 'workspace-form-permission-access-level-label'; diff --git a/src/plugins/workspace/public/components/workspace_detail/data_source_connection_table.scss b/src/plugins/workspace/public/components/workspace_form/data_source_connection_table.scss similarity index 100% rename from src/plugins/workspace/public/components/workspace_detail/data_source_connection_table.scss rename to src/plugins/workspace/public/components/workspace_form/data_source_connection_table.scss diff --git a/src/plugins/workspace/public/components/workspace_detail/data_source_connection_table.tsx b/src/plugins/workspace/public/components/workspace_form/data_source_connection_table.tsx similarity index 63% rename from src/plugins/workspace/public/components/workspace_detail/data_source_connection_table.tsx rename to src/plugins/workspace/public/components/workspace_form/data_source_connection_table.tsx index c11f9fbee254..d440d3e751d6 100644 --- a/src/plugins/workspace/public/components/workspace_detail/data_source_connection_table.tsx +++ b/src/plugins/workspace/public/components/workspace_form/data_source_connection_table.tsx @@ -3,106 +3,55 @@ * SPDX-License-Identifier: Apache-2.0 */ -import './data_source_connection_table.scss'; -import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import React, { useState } from 'react'; import { - EuiSpacer, EuiInMemoryTable, EuiBasicTableColumn, - EuiTableSelectionType, EuiTableActionsColumnType, - EuiConfirmModal, - EuiSearchBarProps, EuiText, EuiListGroup, EuiListGroupItem, EuiPopover, EuiButtonEmpty, EuiPopoverTitle, - EuiSmallButton, EuiLink, EuiButtonIcon, + EuiInMemoryTableProps, + EuiTableSelectionType, } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { DataSourceConnection, DataSourceConnectionType } from '../../../common/types'; import { AssociationDataSourceModalMode } from '../../../common/constants'; -import { DirectQueryConnectionIcon } from '../workspace_form'; +import { DirectQueryConnectionIcon } from './direct_query_connection_icon'; +import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; +import { CoreStart } from '../../../../../core/public'; + +import './data_source_connection_table.scss'; interface DataSourceConnectionTableProps { isDashboardAdmin: boolean; connectionType: string; + onUnlinkDataSource: (dataSources: DataSourceConnection) => void; + onSelectionChange: (selections: DataSourceConnection[]) => void; dataSourceConnections: DataSourceConnection[]; - handleUnassignDataSources: (dataSources: DataSourceConnection[]) => Promise; + tableProps?: Pick, 'pagination' | 'search'>; } export const DataSourceConnectionTable = ({ isDashboardAdmin, connectionType, + onUnlinkDataSource, + onSelectionChange, + tableProps, dataSourceConnections, - handleUnassignDataSources, }: DataSourceConnectionTableProps) => { - const [selectedItems, setSelectedItems] = useState([]); - const [modalVisible, setModalVisible] = useState(false); const [popoversState, setPopoversState] = useState>({}); const [itemIdToExpandedRowMap, setItemIdToExpandedRowMap] = useState< Record >({}); - - useEffect(() => { - // Reset selected items when connectionType changes - setSelectedItems([]); - setItemIdToExpandedRowMap({}); - }, [connectionType]); - - const openSearchConnections = useMemo(() => { - return dataSourceConnections.filter((dsc) => - connectionType === AssociationDataSourceModalMode.OpenSearchConnections - ? dsc.connectionType === DataSourceConnectionType.OpenSearchConnection - : dsc?.relatedConnections && dsc.relatedConnections?.length > 0 - ); - }, [connectionType, dataSourceConnections]); - - const renderToolsLeft = useCallback(() => { - return selectedItems.length > 0 && !modalVisible - ? [ - setModalVisible(true)} - data-test-subj="workspace-detail-dataSources-table-bulkRemove" - > - {i18n.translate('workspace.detail.dataSources.table.remove.button', { - defaultMessage: 'Remove {numberOfSelect} association(s)', - values: { numberOfSelect: selectedItems.length }, - })} - , - ] - : []; - }, [selectedItems, modalVisible]); - - const onSelectionChange = (selectedDataSources: DataSourceConnection[]) => { - setSelectedItems(selectedDataSources); - }; - - const search: EuiSearchBarProps = { - toolsLeft: renderToolsLeft(), - box: { - incremental: true, - }, - filters: [ - { - type: 'field_value_selection', - field: 'type', - name: 'Type', - multiSelect: 'or', - options: Array.from( - new Set(openSearchConnections.map(({ type }) => type).filter(Boolean)) - ).map((type) => ({ - value: type!, - name: type!, - })), - }, - ], - }; + const { + services: { http }, + } = useOpenSearchDashboards(); const togglePopover = (itemId: string) => { setPopoversState((prevState) => ({ @@ -152,19 +101,20 @@ export const DataSourceConnectionTable = ({ ] : []), { - width: '25%', + width: '20%', field: 'name', name: i18n.translate('workspace.detail.dataSources.table.title', { defaultMessage: 'Title', }), truncateText: true, render: (name: string, record) => { - const origin = window.location.origin; let url: string; if (record.connectionType === DataSourceConnectionType.OpenSearchConnection) { - url = `${origin}/app/dataSources/${record.id}`; + url = http.basePath.prepend(`/app/dataSources/${record.id}`); } else { - url = `${origin}/app/dataSources/manage/${name}?dataSourceMDSId=${record.parentId}`; + url = http.basePath.prepend( + `/app/dataSources/manage/${name}?dataSourceMDSId=${record.parentId}` + ); } return ( @@ -174,7 +124,7 @@ export const DataSourceConnectionTable = ({ }, }, { - width: '10%', + width: '20%', field: 'type', name: i18n.translate('workspace.detail.dataSources.table.type', { defaultMessage: 'Type', @@ -182,7 +132,6 @@ export const DataSourceConnectionTable = ({ truncateText: true, }, { - width: '35%', field: 'description', name: i18n.translate('workspace.detail.dataSources.table.description', { defaultMessage: 'Description', @@ -190,11 +139,11 @@ export const DataSourceConnectionTable = ({ truncateText: true, }, { + width: '140px', field: 'relatedConnections', name: i18n.translate('workspace.detail.dataSources.table.relatedConnections', { defaultMessage: 'Related connections', }), - align: 'right', truncateText: true, render: (relatedConnections: DataSourceConnection[], record) => relatedConnections?.length > 0 ? ( @@ -267,12 +216,12 @@ export const DataSourceConnectionTable = ({ icon: 'unlink', type: 'icon', onClick: (item: DataSourceConnection) => { - setSelectedItems([item]); - setModalVisible(true); + onUnlinkDataSource(item); }, 'data-test-subj': 'workspace-detail-dataSources-table-actions-remove', }, ], + width: '10%', } as EuiTableActionsColumnType, ] : []), @@ -284,48 +233,15 @@ export const DataSourceConnectionTable = ({ }; return ( - <> - - - {modalVisible && ( - { - setModalVisible(false); - setSelectedItems([]); - }} - onConfirm={() => { - setModalVisible(false); - handleUnassignDataSources(selectedItems); - }} - cancelButtonText={i18n.translate('workspace.detail.dataSources.modal.cancelButton', { - defaultMessage: 'Cancel', - })} - confirmButtonText={i18n.translate('workspace.detail.dataSources.Modal.confirmButton', { - defaultMessage: 'Remove data source(s)', - })} - buttonColor="danger" - defaultFocusedButton="confirm" - /> - )} - + ); }; diff --git a/src/plugins/workspace/public/components/workspace_form/index.ts b/src/plugins/workspace/public/components/workspace_form/index.ts index 79d934fb80b9..2fae243a37a2 100644 --- a/src/plugins/workspace/public/components/workspace_form/index.ts +++ b/src/plugins/workspace/public/components/workspace_form/index.ts @@ -11,6 +11,7 @@ export { WorkspacePermissionSettingPanel } from './workspace_permission_setting_ export { WorkspaceCancelModal } from './workspace_cancel_modal'; export { WorkspaceNameField, WorkspaceDescriptionField } from './fields'; export { DirectQueryConnectionIcon } from './direct_query_connection_icon'; +export { DataSourceConnectionTable } from './data_source_connection_table'; export { WorkspaceFormSubmitData, WorkspaceFormProps, WorkspaceFormDataState } from './types'; export { @@ -26,6 +27,7 @@ export { export { convertPermissionsToPermissionSettings, convertPermissionSettingsToPermissions, + isWorkspacePermissionSetting, } from './utils'; export { WorkspaceFormProvider, useWorkspaceFormContext } from './workspace_form_context'; diff --git a/src/plugins/workspace/public/components/workspace_form/select_data_source_panel.test.tsx b/src/plugins/workspace/public/components/workspace_form/select_data_source_panel.test.tsx index 2890333f1268..460921530d4b 100644 --- a/src/plugins/workspace/public/components/workspace_form/select_data_source_panel.test.tsx +++ b/src/plugins/workspace/public/components/workspace_form/select_data_source_panel.test.tsx @@ -4,86 +4,229 @@ */ import React from 'react'; -import { fireEvent, render, act } from '@testing-library/react'; -import { SelectDataSourcePanel, SelectDataSourcePanelProps } from './select_data_source_panel'; +import { fireEvent, render, waitFor } from '@testing-library/react'; +import { I18nProvider } from '@osd/i18n/react'; import { coreMock } from '../../../../../core/public/mocks'; +import * as utils from '../../utils'; +import { DataSourceEngineType } from 'src/plugins/data_source/common/data_sources'; +import { OpenSearchDashboardsContextProvider } from '../../../../../plugins/opensearch_dashboards_react/public'; +import { DataSourceConnectionType } from '../../../common/types'; + +import { SelectDataSourcePanel, SelectDataSourcePanelProps } from './select_data_source_panel'; + +const dataSourceConnectionsMock = [ + { + id: 'ds1', + name: 'Data Source 1', + connectionType: DataSourceConnectionType.OpenSearchConnection, + type: 'OpenSearch', + relatedConnections: [ + { + id: 'ds1-dqc1', + name: 'dqc1', + parentId: 'ds1', + connectionType: DataSourceConnectionType.DirectQueryConnection, + type: 'Amazon S3', + }, + ], + }, + { + id: 'ds1-dqc1', + name: 'dqc1', + parentId: 'ds1', + connectionType: DataSourceConnectionType.DirectQueryConnection, + type: 'Amazon S3', + }, + { + id: 'ds2', + name: 'Data Source 2', + connectionType: DataSourceConnectionType.OpenSearchConnection, + type: 'OpenSearch', + }, +]; + +const assignedDataSourcesConnections = [dataSourceConnectionsMock[0], dataSourceConnectionsMock[2]]; const dataSources = [ { - id: 'id1', - title: 'title1', + id: 'ds1', + title: 'Data Source 1', + description: 'Description of data source 1', + auth: '', + dataSourceEngineType: '' as DataSourceEngineType, + workspaces: [], + }, + { + id: 'ds2', + title: 'Data Source 2', + description: 'Description of data source 2', + auth: '', + dataSourceEngineType: '' as DataSourceEngineType, + workspaces: [], }, - { id: 'id2', title: 'title2' }, ]; -jest.mock('../../utils', () => ({ - getDataSourcesList: jest.fn().mockResolvedValue(dataSources), -})); +jest.spyOn(utils, 'getDataSourcesList').mockResolvedValue(dataSources); +jest.spyOn(utils, 'fetchDataSourceConnections').mockImplementation(async (passedDataSources) => { + return dataSourceConnectionsMock.filter(({ id }) => + passedDataSources.some((dataSource) => dataSource.id === id) + ); +}); const mockCoreStart = coreMock.createStart(); const setup = ({ savedObjects = mockCoreStart.savedObjects, - selectedDataSources = [], + assignedDataSourceConnections = [], onChange = jest.fn(), errors = undefined, + showDataSourceManagement = true, }: Partial) => { return render( - + + + + + ); }; describe('SelectDataSourcePanel', () => { - it('should render consistent data sources when selected data sources passed', () => { - const { getByText } = setup({ selectedDataSources: dataSources }); + const originalOffsetHeight = Object.getOwnPropertyDescriptor( + HTMLElement.prototype, + 'offsetHeight' + ); + const originalOffsetWidth = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'offsetWidth'); + beforeEach(() => { + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { + configurable: true, + value: 600, + }); + Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { + configurable: true, + value: 600, + }); + }); + afterEach(() => { + Object.defineProperty( + HTMLElement.prototype, + 'offsetHeight', + originalOffsetHeight as PropertyDescriptor + ); + Object.defineProperty( + HTMLElement.prototype, + 'offsetWidth', + originalOffsetWidth as PropertyDescriptor + ); + }); + it('should render consistent data sources when selected data sources passed', async () => { + const { getByText, getByTestId, queryByText } = setup({ + assignedDataSourceConnections: [assignedDataSourcesConnections[0]], + }); + + await waitFor(() => { + expect(getByText(assignedDataSourcesConnections[0].name)).toBeInTheDocument(); + expect(queryByText(assignedDataSourcesConnections[1].name)).not.toBeInTheDocument(); + }); - expect(getByText(dataSources[0].title)).toBeInTheDocument(); - expect(getByText(dataSources[1].title)).toBeInTheDocument(); + fireEvent.click(getByTestId('workspace-creator-dataSources-assign-button')); + + await waitFor(() => { + expect(getByText(assignedDataSourcesConnections[1].name)).toBeInTheDocument(); + }); }); - it('should call onChange when clicking add new data source button', () => { + it('should call onChange when updating data sources', async () => { const onChangeMock = jest.fn(); - const { getByTestId } = setup({ onChange: onChangeMock }); + const { getByTestId, getByText } = setup({ + onChange: onChangeMock, + assignedDataSourceConnections: [], + }); expect(onChangeMock).not.toHaveBeenCalled(); - fireEvent.click(getByTestId('workspaceForm-select-dataSource-addNew')); + fireEvent.click(getByTestId('workspace-creator-dataSources-assign-button')); + + await waitFor(() => { + expect( + getByText( + 'Add data sources that will be available in the workspace. If a selected data source has related Direct Query connection, they will also be available in the workspace.' + ) + ).toBeInTheDocument(); + expect(getByText(assignedDataSourcesConnections[1].name)).toBeInTheDocument(); + }); + + fireEvent.click(getByText(assignedDataSourcesConnections[1].name)); + fireEvent.click(getByText('Associate data sources')); expect(onChangeMock).toHaveBeenCalledWith([ - { - id: '', - title: '', - }, + expect.objectContaining({ + id: assignedDataSourcesConnections[1].id, + }), + ]); + + fireEvent.click(getByTestId('workspace-creator-dqc-assign-button')); + await waitFor(() => { + expect(getByText(assignedDataSourcesConnections[0].name)).toBeInTheDocument(); + }); + fireEvent.click(getByText(assignedDataSourcesConnections[0].name)); + fireEvent.click(getByText('Associate data sources')); + expect(onChangeMock).toHaveBeenCalledWith([ + expect.objectContaining({ + id: assignedDataSourcesConnections[0].id, + }), ]); }); - it('should call onChange when updating selected data sources in combo box', async () => { + it('should call onChange when deleting selected data source', async () => { const onChangeMock = jest.fn(); - const { getByTitle, getByText } = setup({ + const { getByText, getByTestId } = setup({ onChange: onChangeMock, - selectedDataSources: [{ id: '', title: '' }], + assignedDataSourceConnections: assignedDataSourcesConnections, + }); + fireEvent.click(getByTestId('workspace-creator-dataSources-assign-button')); + + await waitFor(() => { + expect(getByText(assignedDataSourcesConnections[0].name)).toBeInTheDocument(); + expect(getByText(assignedDataSourcesConnections[1].name)).toBeInTheDocument(); }); + + fireEvent.click(getByText(assignedDataSourcesConnections[0].name)); + fireEvent.click(getByText(assignedDataSourcesConnections[1].name)); + expect(onChangeMock).not.toHaveBeenCalled(); - await act(() => { - fireEvent.click(getByText('Select')); + + fireEvent.click(getByText('Associate data sources')); + + await waitFor(() => { + fireEvent.click(getByTestId('checkboxSelectRow-' + dataSources[1].id)); + fireEvent.click(getByText('Remove selected')); }); - fireEvent.click(getByTitle(dataSources[0].title)); - expect(onChangeMock).toHaveBeenCalledWith([{ id: 'id1', title: 'title1' }]); + expect(onChangeMock).toHaveBeenCalledWith([assignedDataSourcesConnections[0]]); }); - it('should call onChange when deleting selected data source', async () => { - const onChangeMock = jest.fn(); - const { getByLabelText } = setup({ - onChange: onChangeMock, - selectedDataSources: [{ id: '', title: '' }], + it('should close associate data sources modal', async () => { + const { getByText, queryByText, getByTestId } = setup({ + assignedDataSourceConnections: [], }); - expect(onChangeMock).not.toHaveBeenCalled(); - await act(() => { - fireEvent.click(getByLabelText('Delete data source')); + + fireEvent.click(getByTestId('workspace-creator-dataSources-assign-button')); + await waitFor(() => { + expect( + getByText( + 'Add data sources that will be available in the workspace. If a selected data source has related Direct Query connection, they will also be available in the workspace.' + ) + ).toBeInTheDocument(); }); - expect(onChangeMock).toHaveBeenCalledWith([]); + fireEvent.click(getByText('Close')); + expect( + queryByText( + 'Add data sources that will be available in the workspace. If a selected data source has related Direct Query connection, they will also be available in the workspace.' + ) + ).toBeNull(); }); }); diff --git a/src/plugins/workspace/public/components/workspace_form/select_data_source_panel.tsx b/src/plugins/workspace/public/components/workspace_form/select_data_source_panel.tsx index 9c990d7e7195..b1ec518d77c1 100644 --- a/src/plugins/workspace/public/components/workspace_form/select_data_source_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_form/select_data_source_panel.tsx @@ -3,149 +3,149 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useCallback, useEffect, useState } from 'react'; -import { - EuiSmallButton, - EuiCompressedFormRow, - EuiSpacer, - EuiFlexGroup, - EuiFlexItem, - EuiButtonIcon, - EuiCompressedComboBox, - EuiComboBoxOptionOption, - EuiFormLabel, -} from '@elastic/eui'; +import React, { useState } from 'react'; +import { EuiSpacer, EuiFlexItem, EuiSmallButton, EuiFlexGroup, EuiPanel } from '@elastic/eui'; import { i18n } from '@osd/i18n'; -import { SavedObjectsStart } from '../../../../../core/public'; -import { getDataSourcesList } from '../../utils'; -import { DataSource } from '../../../common/types'; +import { SavedObjectsStart, CoreStart } from '../../../../../core/public'; +import { DataSourceConnection } from '../../../common/types'; import { WorkspaceFormError } from './types'; +import { AssociationDataSourceModal } from '../workspace_detail/association_data_source_modal'; +import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; +import { WorkspaceClient } from '../../workspace_client'; +import { AssociationDataSourceModalMode } from '../../../common/constants'; +import { DataSourceConnectionTable } from './data_source_connection_table'; export interface SelectDataSourcePanelProps { errors?: { [key: number]: WorkspaceFormError }; savedObjects: SavedObjectsStart; - selectedDataSources: DataSource[]; - onChange: (value: DataSource[]) => void; + assignedDataSourceConnections: DataSourceConnection[]; + onChange: (value: DataSourceConnection[]) => void; + showDataSourceManagement: boolean; } export const SelectDataSourcePanel = ({ - errors, onChange, - selectedDataSources, + assignedDataSourceConnections, savedObjects, + showDataSourceManagement, }: SelectDataSourcePanelProps) => { - const [dataSourcesOptions, setDataSourcesOptions] = useState([]); - useEffect(() => { - if (!savedObjects) return; - getDataSourcesList(savedObjects.client, ['*']).then((result) => { - const options = result.map(({ title, id }) => ({ - label: title, - value: id, - })); - setDataSourcesOptions(options); - }); - }, [savedObjects, setDataSourcesOptions]); - const handleAddNewOne = useCallback(() => { - onChange?.([ - ...selectedDataSources, - { - title: '', - id: '', - }, - ]); - }, [onChange, selectedDataSources]); + const [modalVisible, setModalVisible] = useState(false); + const [selectedItems, setSelectedItems] = useState([]); + const [toggleIdSelected, setToggleIdSelected] = useState( + AssociationDataSourceModalMode.OpenSearchConnections + ); + const { + services: { notifications, http, chrome }, + } = useOpenSearchDashboards<{ CoreStart: CoreStart; workspaceClient: WorkspaceClient }>(); + + const handleAssignDataSourceConnections = (newDataSourceConnections: DataSourceConnection[]) => { + setModalVisible(false); + onChange([...assignedDataSourceConnections, ...newDataSourceConnections]); + }; + + const handleUnassignDataSources = (dataSourceConnections: DataSourceConnection[]) => { + onChange( + assignedDataSourceConnections.filter( + ({ id }: DataSourceConnection) => !dataSourceConnections.some((item) => item.id === id) + ) + ); + }; - const handleSelect = useCallback( - (selectedOptions, index) => { - const newOption = selectedOptions[0] - ? // Select new data source - { - title: selectedOptions[0].label, - id: selectedOptions[0].value, - } - : // Click reset button - { - title: '', - id: '', - }; - const newSelectedOptions = [...selectedDataSources]; - newSelectedOptions.splice(index, 1, newOption); + const handleSingleDataSourceUnAssign = (connection: DataSourceConnection) => { + handleUnassignDataSources([connection]); + }; - onChange(newSelectedOptions); - }, - [onChange, selectedDataSources] + const renderTableContent = () => { + return ( + + + + ); + }; + + const addOpenSearchConnectionsButton = ( + { + setToggleIdSelected(AssociationDataSourceModalMode.OpenSearchConnections); + setModalVisible(true); + }} + data-test-subj="workspace-creator-dataSources-assign-button" + > + {i18n.translate('workspace.form.selectDataSourcePanel.addNew', { + defaultMessage: 'Add OpenSearch connections', + })} + ); - const handleDelete = useCallback( - (index) => { - const newSelectedOptions = [...selectedDataSources]; - newSelectedOptions.splice(index, 1); + const addDirectQueryConnectionsButton = ( + { + setToggleIdSelected(AssociationDataSourceModalMode.DirectQueryConnections); + setModalVisible(true); + }} + data-test-subj="workspace-creator-dqc-assign-button" + > + {i18n.translate('workspace.form.selectDataSourcePanel.addNewDQCs', { + defaultMessage: 'Add direct query connections', + })} + + ); - onChange(newSelectedOptions); - }, - [onChange, selectedDataSources] + const removeButton = ( + { + handleUnassignDataSources(selectedItems); + }} + data-test-subj="workspace-creator-dataSources-remove-button" + > + {i18n.translate('workspace.form.selectDataSourcePanel.remove', { + defaultMessage: 'Remove selected', + })} + ); return (
- - {i18n.translate('workspace.form.selectDataSource.subTitle', { - defaultMessage: 'Data source', - })} - - - {selectedDataSources.map(({ id, title }, index) => ( - - - - handleSelect(selectedOptions, index)} - placeholder="Select" - /> - - - handleDelete(index)} - isDisabled={false} - /> - - - - ))} - - - {i18n.translate('workspace.form.selectDataSourcePanel.addNew', { - defaultMessage: 'Add New', - })} - + + + {showDataSourceManagement && + selectedItems.length > 0 && + assignedDataSourceConnections.length > 0 && ( + {removeButton} + )} + {showDataSourceManagement && ( + {addOpenSearchConnectionsButton} + )} + {showDataSourceManagement && ( + {addDirectQueryConnectionsButton} + )} + + + + {assignedDataSourceConnections.length > 0 && renderTableContent()} + + {modalVisible && chrome && ( + setModalVisible(false)} + handleAssignDataSourceConnections={handleAssignDataSourceConnections} + http={http} + mode={toggleIdSelected} + notifications={notifications} + logos={chrome.logos} + /> + )}
); }; diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts index 0bf8881cd3bd..7ca175f794be 100644 --- a/src/plugins/workspace/public/components/workspace_form/types.ts +++ b/src/plugins/workspace/public/components/workspace_form/types.ts @@ -6,7 +6,7 @@ import type { ApplicationStart, SavedObjectsStart } from '../../../../../core/public'; import type { WorkspacePermissionMode } from '../../../common/constants'; import type { WorkspaceOperationType, WorkspacePermissionItemType } from './constants'; -import { DataSource } from '../../../common/types'; +import { DataSourceConnection } from '../../../common/types'; import { DataSourceManagementPluginSetup } from '../../../../../plugins/data_source_management/public'; import { WorkspaceUseCase } from '../../types'; @@ -34,7 +34,7 @@ export interface WorkspaceFormSubmitData { features: string[]; color?: string; permissionSettings?: WorkspacePermissionSetting[]; - selectedDataSources?: DataSource[]; + selectedDataSourceConnections?: DataSourceConnection[]; } export enum WorkspaceFormErrorCode { @@ -61,14 +61,14 @@ export interface WorkspaceFormError { export type WorkspaceFormErrors = { [key in keyof Omit< WorkspaceFormSubmitData, - 'permissionSettings' | 'description' | 'selectedDataSources' + 'permissionSettings' | 'description' | 'selectedDataSourceConnections' >]?: WorkspaceFormError; } & { permissionSettings?: { overall?: WorkspaceFormError; fields?: { [key: number]: WorkspaceFormError }; }; - selectedDataSources?: { [key: number]: WorkspaceFormError }; + selectedDataSourceConnections?: { [key: number]: WorkspaceFormError }; }; export interface WorkspaceFormProps { @@ -91,7 +91,7 @@ export interface WorkspaceFormDataState extends Omit { name: string; useCase: string | undefined; - selectedDataSources: DataSource[]; + selectedDataSourceConnections: DataSourceConnection[]; permissionSettings: Array< Pick & Partial >; diff --git a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts index 703255455ec4..74cf11982f4d 100644 --- a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts +++ b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts @@ -8,7 +8,7 @@ import { htmlIdGenerator, EuiColorPickerProps } from '@elastic/eui'; import { useApplications } from '../../hooks'; import { getFirstUseCaseOfFeatureConfigs, isUseCaseFeatureConfig } from '../../utils'; -import { DataSource } from '../../../common/types'; +import { DataSourceConnection } from '../../../common/types'; import { getUseCaseFeatureConfig } from '../../../common/utils'; import { WorkspaceFormProps, @@ -51,9 +51,12 @@ export const useWorkspaceForm = ({ WorkspaceFormDataState['permissionSettings'] >(initialPermissionSettingsRef.current); - const [selectedDataSources, setSelectedDataSources] = useState( - defaultValues?.selectedDataSources && defaultValues.selectedDataSources.length > 0 - ? defaultValues.selectedDataSources + const [selectedDataSourceConnections, setSelectedDataSourceConnections] = useState< + DataSourceConnection[] + >( + defaultValues?.selectedDataSourceConnections && + defaultValues.selectedDataSourceConnections.length > 0 + ? defaultValues.selectedDataSourceConnections : [] ); @@ -67,7 +70,7 @@ export const useWorkspaceForm = ({ useCase: selectedUseCase, color, permissionSettings, - selectedDataSources, + selectedDataSourceConnections, }); const getFormDataRef = useRef(getFormData); getFormDataRef.current = getFormData; @@ -122,7 +125,7 @@ export const useWorkspaceForm = ({ color: currentFormData.color || '#FFFFFF', features: currentFormData.features, permissionSettings: currentFormData.permissionSettings as WorkspacePermissionSetting[], - selectedDataSources: currentFormData.selectedDataSources, + selectedDataSourceConnections: currentFormData.selectedDataSourceConnections, }); }, [onSubmit, permissionEnabled] @@ -159,6 +162,6 @@ export const useWorkspaceForm = ({ handleColorChange, handleUseCaseChange, setPermissionSettings, - setSelectedDataSources, + setSelectedDataSourceConnections, }; }; diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts index 3a45165044d7..03cea502f573 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.test.ts @@ -9,9 +9,15 @@ import { convertPermissionsToPermissionSettings, getNumberOfChanges, getNumberOfErrors, + isWorkspacePermissionSetting, } from './utils'; import { WorkspacePermissionMode } from '../../../common/constants'; -import { WorkspacePermissionItemType } from './constants'; +import { + WorkspacePermissionItemType, + optionIdToWorkspacePermissionModesMap, + PermissionModeId, +} from './constants'; +import { DataSourceConnectionType } from '../../../common/types'; import { WorkspaceFormErrorCode } from './types'; describe('convertPermissionSettingsToPermissions', () => { @@ -337,15 +343,17 @@ describe('validateWorkspaceForm', () => { validateWorkspaceForm( { name: 'test', - selectedDataSources: [ + selectedDataSourceConnections: [ { id: '', - title: 'title', + name: 'title', + connectionType: DataSourceConnectionType.OpenSearchConnection, + type: 'OpenSearch', }, ], }, false - ).selectedDataSources + ).selectedDataSourceConnections ).toEqual({ 0: { code: WorkspaceFormErrorCode.InvalidDataSource, message: 'Invalid data source' }, }); @@ -356,19 +364,23 @@ describe('validateWorkspaceForm', () => { validateWorkspaceForm( { name: 'test', - selectedDataSources: [ + selectedDataSourceConnections: [ { id: 'id', - title: 'title1', + name: 'title1', + connectionType: DataSourceConnectionType.OpenSearchConnection, + type: 'OpenSearch', }, { id: 'id', - title: 'title2', + name: 'title2', + connectionType: DataSourceConnectionType.OpenSearchConnection, + type: 'OpenSearch', }, ], }, false - ).selectedDataSources + ).selectedDataSourceConnections ).toEqual({ '1': { code: WorkspaceFormErrorCode.DuplicateDataSource, message: 'Duplicate data sources' }, }); @@ -379,7 +391,7 @@ describe('getNumberOfErrors', () => { it('should calculate the error number of data sources form', () => { expect( getNumberOfErrors({ - selectedDataSources: { + selectedDataSourceConnections: { 0: { code: WorkspaceFormErrorCode.InvalidDataSource, message: 'Invalid data source' }, }, }) @@ -617,3 +629,72 @@ describe('getNumberOfChanges', () => { ).toEqual(3); }); }); + +describe('isWorkspacePermissionSetting', () => { + it('should return true for a valid user permission setting', () => { + const validUserPermissionSetting = { + modes: optionIdToWorkspacePermissionModesMap[PermissionModeId.Read], + type: WorkspacePermissionItemType.User, + userId: 'user123', + }; + const result = isWorkspacePermissionSetting(validUserPermissionSetting); + expect(result).toBe(true); + }); + + it('should return true for a valid group permission setting', () => { + const validGroupPermissionSetting = { + modes: optionIdToWorkspacePermissionModesMap[PermissionModeId.Owner], + type: WorkspacePermissionItemType.Group, + group: 'group456', + }; + const result = isWorkspacePermissionSetting(validGroupPermissionSetting); + expect(result).toBe(true); + }); + + it('should return false if modes is missing', () => { + const permissionSettingWithoutModes = { + type: WorkspacePermissionItemType.User, + userId: 'user123', + }; + const result = isWorkspacePermissionSetting(permissionSettingWithoutModes); + expect(result).toBe(false); + }); + + it('should return false if modes are invalid', () => { + const permissionSettingWithInvalidModes = { + modes: ['invalid' as WorkspacePermissionMode], + type: WorkspacePermissionItemType.User, + userId: 'user123', + }; + const result = isWorkspacePermissionSetting(permissionSettingWithInvalidModes); + expect(result).toBe(false); + }); + + it('should return false if type is invalid', () => { + const permissionSettingWithInvalidType = { + modes: optionIdToWorkspacePermissionModesMap[PermissionModeId.Owner], + type: 'invalid', + userId: 'user123', + }; + const result = isWorkspacePermissionSetting(permissionSettingWithInvalidType); + expect(result).toBe(false); + }); + + it('should return false if userId is missing for user type', () => { + const permissionSettingWithoutUserId = { + modes: optionIdToWorkspacePermissionModesMap[PermissionModeId.Owner], + type: WorkspacePermissionItemType.User, + }; + const result = isWorkspacePermissionSetting(permissionSettingWithoutUserId); + expect(result).toBe(false); + }); + + it('should return false if group is missing for group type', () => { + const permissionSettingWithoutGroup = { + modes: optionIdToWorkspacePermissionModesMap[PermissionModeId.Owner], + type: WorkspacePermissionItemType.Group, + }; + const result = isWorkspacePermissionSetting(permissionSettingWithoutGroup); + expect(result).toBe(false); + }); +}); diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index 161def1f3f6b..5e03c724889a 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -25,7 +25,7 @@ import { WorkspaceUserGroupPermissionSetting, WorkspaceUserPermissionSetting, } from './types'; -import { DataSource } from '../../../common/types'; +import { DataSourceConnection } from '../../../common/types'; import { validateWorkspaceColor } from '../../../common/utils'; export const isValidFormTextInput = (input?: string) => { @@ -48,8 +48,8 @@ export const getNumberOfErrors = (formErrors: WorkspaceFormErrors) => { if (formErrors.permissionSettings?.overall) { numberOfErrors += 1; } - if (formErrors.selectedDataSources) { - numberOfErrors += Object.keys(formErrors.selectedDataSources).length; + if (formErrors.selectedDataSourceConnections) { + numberOfErrors += Object.keys(formErrors.selectedDataSourceConnections).length; } if (formErrors.features) { numberOfErrors += 1; @@ -226,7 +226,7 @@ const validateUserGroupPermissionSetting = ( } }; -const validatePermissionSetting = ( +const validatePermissionSettings = ( permissionSettings?: Array< Pick & Partial > @@ -289,17 +289,17 @@ const validatePermissionSetting = ( : {}), }; }; -export const isSelectedDataSourcesDuplicated = ( - selectedDataSources: DataSource[], - row: DataSource -) => selectedDataSources.some((ds) => ds.id === row.id); +export const isSelectedDataSourceConnectionsDuplicated = ( + selectedDataSourceConnections: DataSourceConnection[], + row: DataSourceConnection +) => selectedDataSourceConnections.some((connection) => connection.id === row.id); export const validateWorkspaceForm = ( formData: Partial, isPermissionEnabled: boolean ) => { const formErrors: WorkspaceFormErrors = {}; - const { name, permissionSettings, color, features, selectedDataSources } = formData; + const { name, permissionSettings, color, features, selectedDataSourceConnections } = formData; if (name && name.trim()) { if (!isValidFormTextInput(name)) { formErrors.name = { @@ -334,12 +334,12 @@ export const validateWorkspaceForm = ( }; } if (isPermissionEnabled) { - formErrors.permissionSettings = validatePermissionSetting(permissionSettings); + formErrors.permissionSettings = validatePermissionSettings(permissionSettings); } - if (selectedDataSources) { + if (selectedDataSourceConnections) { const dataSourcesErrors: { [key: number]: WorkspaceFormError } = {}; - for (let i = 0; i < selectedDataSources.length; i++) { - const row = selectedDataSources[i]; + for (let i = 0; i < selectedDataSourceConnections.length; i++) { + const row = selectedDataSourceConnections[i]; if (!row.id) { dataSourcesErrors[i] = { code: WorkspaceFormErrorCode.InvalidDataSource, @@ -347,7 +347,9 @@ export const validateWorkspaceForm = ( defaultMessage: 'Invalid data source', }), }; - } else if (isSelectedDataSourcesDuplicated(selectedDataSources.slice(0, i), row)) { + } else if ( + isSelectedDataSourceConnectionsDuplicated(selectedDataSourceConnections.slice(0, i), row) + ) { dataSourcesErrors[i] = { code: WorkspaceFormErrorCode.DuplicateDataSource, message: i18n.translate('workspace.form.permission.invalidate.group', { @@ -357,7 +359,7 @@ export const validateWorkspaceForm = ( } } if (Object.keys(dataSourcesErrors).length > 0) { - formErrors.selectedDataSources = dataSourcesErrors; + formErrors.selectedDataSourceConnections = dataSourcesErrors; } } return formErrors; @@ -447,6 +449,34 @@ const isSamePermissionSetting = (a: PermissionSettingLike, b: PermissionSettingL ); }; +export const isWorkspacePermissionSetting = ( + permissionSetting: PermissionSettingLike +): permissionSetting is WorkspacePermissionSetting => { + const { modes, type, userId, group } = permissionSetting; + if (!modes) { + return false; + } + const arrayStringify = (array: string[]) => array.sort().join(); + const stringifyModes = arrayStringify(modes); + if ( + Object.values(optionIdToWorkspacePermissionModesMap).every( + (validModes) => arrayStringify([...validModes]) !== stringifyModes + ) + ) { + return false; + } + if (type !== WorkspacePermissionItemType.User && type !== WorkspacePermissionItemType.Group) { + return false; + } + if (type === WorkspacePermissionItemType.User && !userId) { + return false; + } + if (type === WorkspacePermissionItemType.Group && !group) { + return false; + } + return true; +}; + export const getNumberOfChanges = ( newFormData: Partial, initialFormData: Partial diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_context.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_context.tsx index 417921f170d3..9ac317f1ec07 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form_context.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_context.tsx @@ -5,16 +5,17 @@ import React, { createContext, useContext, FormEventHandler, ReactNode } from 'react'; import { EuiColorPickerOutput } from '@elastic/eui/src/components/color_picker/color_picker'; -import { DataSource } from '../../../common/types'; -import { WorkspaceFormProps, WorkspaceFormErrors, WorkspacePermissionSetting } from './types'; +import { DataSourceConnection } from '../../../common/types'; +import { WorkspaceFormProps, WorkspaceFormErrors } from './types'; import { PublicAppInfo } from '../../../../../core/public'; import { useWorkspaceForm } from './use_workspace_form'; +import { WorkspaceFormDataState } from '../workspace_form'; interface WorkspaceFormContextProps { formId: string; setName: React.Dispatch>; setDescription: React.Dispatch>; - formData: any; + formData: WorkspaceFormDataState; isEditing: boolean; formErrors: WorkspaceFormErrors; setIsEditing: React.Dispatch>; @@ -26,11 +27,9 @@ interface WorkspaceFormContextProps { handleColorChange: (text: string, output: EuiColorPickerOutput) => void; handleUseCaseChange: (newUseCase: string) => void; setPermissionSettings: React.Dispatch< - React.SetStateAction< - Array & Partial> - > + React.SetStateAction >; - setSelectedDataSources: React.Dispatch>; + setSelectedDataSourceConnections: React.Dispatch>; } const initialContextValue: WorkspaceFormContextProps = {} as WorkspaceFormContextProps; diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx index 00560f7c033d..86f0d0688714 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx @@ -18,6 +18,9 @@ import { WorkspacePermissionItemType, optionIdToWorkspacePermissionModesMap, PermissionModeId, + PERMISSION_TYPE_LABEL_ID, + PERMISSION_COLLABORATOR_LABEL_ID, + PERMISSION_ACCESS_LEVEL_LABEL_ID, } from './constants'; import { getPermissionModeId } from './utils'; @@ -148,6 +151,7 @@ export const WorkspacePermissionSettingInput = ({ onChange={(value) => onTypeChange(value, index)} disabled={userOrGroupDisabled || !isEditing} data-test-subj="workspace-typeOptions" + aria-labelledby={PERMISSION_TYPE_LABEL_ID} /> @@ -166,6 +170,7 @@ export const WorkspacePermissionSettingInput = ({ defaultMessage: 'Enter group name or group ID', }) } + aria-labelledby={PERMISSION_COLLABORATOR_LABEL_ID} /> @@ -176,6 +181,7 @@ export const WorkspacePermissionSettingInput = ({ onChange={handlePermissionModeOptionChange} disabled={userOrGroupDisabled || !isEditing} data-test-subj="workspace-permissionModeOptions" + aria-labelledby={PERMISSION_ACCESS_LEVEL_LABEL_ID} /> diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx index 8ea255b83b36..845708d7ecbf 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx @@ -10,6 +10,7 @@ import { EuiFlexItem, EuiCompressedFormRow, EuiSpacer, + EuiFormLabel, } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { WorkspaceFormError, WorkspacePermissionSetting } from './types'; @@ -17,6 +18,9 @@ import { WorkspacePermissionItemType, optionIdToWorkspacePermissionModesMap, PermissionModeId, + PERMISSION_TYPE_LABEL_ID, + PERMISSION_COLLABORATOR_LABEL_ID, + PERMISSION_ACCESS_LEVEL_LABEL_ID, } from './constants'; import { WorkspacePermissionSettingInput, @@ -130,35 +134,30 @@ export const WorkspacePermissionSettingPanel = ({ ); return ( -
- + <> + - + {i18n.translate('workspaceForm.permissionSetting.typeLabel', { defaultMessage: 'Type', })} - > - <> - + - + {i18n.translate('workspaceForm.permissionSetting.collaboratorLabel', { defaultMessage: 'Collaborator', })} - > - <> - + - - + + {i18n.translate('workspaceForm.permissionSetting.accessLevelLabel', { defaultMessage: 'Access level', })} - > - <> - + + {permissionSettings.map((item, index) => ( @@ -195,6 +194,6 @@ export const WorkspacePermissionSettingPanel = ({ })} )} -
+ ); }; diff --git a/src/plugins/workspace/public/utils.ts b/src/plugins/workspace/public/utils.ts index bbc3d0dbd6a1..49132b64f467 100644 --- a/src/plugins/workspace/public/utils.ts +++ b/src/plugins/workspace/public/utils.ts @@ -266,28 +266,46 @@ export const getDirectQueryConnections = async (dataSourceId: string, http: Http return directQueryConnections; }; -// Helper function to merge data sources with direct query connections -export const mergeDataSourcesWithConnections = ( - assignedDataSources: DataSource[], - directQueryConnections: DataSourceConnection[] -): DataSourceConnection[] => { - const dataSources: DataSourceConnection[] = []; - assignedDataSources.forEach((ds) => { - const relatedConnections = directQueryConnections.filter( - (directQueryConnection) => directQueryConnection.parentId === ds.id - ); - - dataSources.push({ +export const convertDataSourcesToOpenSearchConnections = ( + dataSources: DataSource[] +): DataSourceConnection[] => + dataSources.map((ds) => { + return { id: ds.id, type: ds.dataSourceEngineType, connectionType: DataSourceConnectionType.OpenSearchConnection, name: ds.title, description: ds.description, + relatedConnections: [], + }; + }); + +export const fulfillRelatedConnections = ( + connections: DataSourceConnection[], + directQueryConnections: DataSourceConnection[] +) => { + return connections.map((connection) => { + const relatedConnections = directQueryConnections.filter( + (directQueryConnection) => directQueryConnection.parentId === connection.id + ); + return { + ...connection, relatedConnections, - }); + }; }); +}; - return [...dataSources, ...directQueryConnections]; +// Helper function to merge data sources with direct query connections +export const mergeDataSourcesWithConnections = ( + dataSources: DataSource[], + directQueryConnections: DataSourceConnection[] +): DataSourceConnection[] => { + const openSearchConnections = convertDataSourcesToOpenSearchConnections(dataSources); + + return [ + ...fulfillRelatedConnections(openSearchConnections, directQueryConnections), + ...directQueryConnections, + ].sort((a, b) => a.name.localeCompare(b.name)); }; // If all connected data sources are serverless, will only allow to select essential use case. @@ -475,21 +493,28 @@ export const getUseCaseUrl = ( return useCaseURL; }; +export const fetchDataSourceConnectionsByDataSourceIds = async ( + dataSourceIds: string[], + http: HttpSetup | undefined +) => { + const directQueryConnectionsPromises = dataSourceIds.map((dataSourceId) => + getDirectQueryConnections(dataSourceId, http!).catch(() => []) + ); + const directQueryConnectionsResult = await Promise.all(directQueryConnectionsPromises); + return directQueryConnectionsResult.flat(); +}; + export const fetchDataSourceConnections = async ( assignedDataSources: DataSource[], http: HttpSetup | undefined, notifications: NotificationsStart | undefined ) => { try { - const directQueryConnectionsPromises = assignedDataSources.map((ds) => - getDirectQueryConnections(ds.id, http!).catch(() => []) + const directQueryConnections = await fetchDataSourceConnectionsByDataSourceIds( + assignedDataSources.map((ds) => ds.id), + http ); - const directQueryConnectionsResult = await Promise.all(directQueryConnectionsPromises); - const directQueryConnections = directQueryConnectionsResult.flat(); - return mergeDataSourcesWithConnections( - assignedDataSources, - directQueryConnections - ).sort((a, b) => a.name.localeCompare(b.name)); + return mergeDataSourcesWithConnections(assignedDataSources, directQueryConnections); } catch (error) { notifications?.toasts.addDanger( i18n.translate('workspace.detail.dataSources.error.message', { From 80de45ef8ca7de951e7ecd60d79378a8494056b6 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Fri, 6 Sep 2024 00:26:09 +0800 Subject: [PATCH 09/12] fix 2.x issue (#8021) (#8022) (cherry picked from commit bb6a3bc387b94ec1cf70241f989eb11444cc87dd) Signed-off-by: Hailong Cui Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- src/core/public/chrome/ui/header/header_breadcrumbs.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/public/chrome/ui/header/header_breadcrumbs.tsx b/src/core/public/chrome/ui/header/header_breadcrumbs.tsx index 35ef40f0209f..ca0f3610641d 100644 --- a/src/core/public/chrome/ui/header/header_breadcrumbs.tsx +++ b/src/core/public/chrome/ui/header/header_breadcrumbs.tsx @@ -75,7 +75,7 @@ export function HeaderBreadcrumbs({ crumbs = breadcrumbEnricher(crumbs); } - if (dropHomeFromBreadcrumb && crumbs.length && Object.hasOwn(crumbs[0], 'home')) { + if (dropHomeFromBreadcrumb && crumbs.length && crumbs[0].hasOwnProperty('home')) { crumbs = crumbs.slice(1); } From 06816d59e10ac2bb9ad93f9f89ec9a791962f228 Mon Sep 17 00:00:00 2001 From: Kawika Avilla Date: Thu, 5 Sep 2024 13:16:04 -0700 Subject: [PATCH 10/12] [discover] undefined datasource fix for index patterns for PPL and SQL(#8027) Deconstructing the dataSource caused an exception when no dataSource was set with the query. This can be the case with index patterns that are created and pointed to the default. Most times the default is the local cluster so there is no data source object in the query the case of the local. Accessing by dot notation with null operator fixes this issue. Adding tests to prevent this happening again. With query enhancements enabled we should considered ensuring index patterns always have a datasource object in the query. Signed-off-by: Kawika Avilla --- .../server/utils/facet.test.ts | 119 ++++++++++++++++++ .../query_enhancements/server/utils/facet.ts | 4 +- 2 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 src/plugins/query_enhancements/server/utils/facet.test.ts diff --git a/src/plugins/query_enhancements/server/utils/facet.test.ts b/src/plugins/query_enhancements/server/utils/facet.test.ts new file mode 100644 index 000000000000..20ae78612c11 --- /dev/null +++ b/src/plugins/query_enhancements/server/utils/facet.test.ts @@ -0,0 +1,119 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Logger } from 'opensearch-dashboards/server'; +import { Facet, FacetProps } from './facet'; + +describe('Facet', () => { + let facet: Facet; + let mockClient: jest.Mock; + let mockLogger: jest.Mocked; + let mockContext: any; + let mockRequest: any; + + beforeEach(() => { + mockClient = jest.fn(); + mockLogger = ({ + error: jest.fn(), + } as unknown) as jest.Mocked; + + const props: FacetProps = { + client: { asScoped: jest.fn().mockReturnValue({ callAsCurrentUser: mockClient }) }, + logger: mockLogger, + endpoint: 'test-endpoint', + }; + + facet = new Facet(props); + + mockContext = { + dataSource: { + opensearch: { + legacy: { + getClient: jest.fn().mockReturnValue({ callAPI: mockClient }), + }, + }, + }, + }; + + mockRequest = { + body: { + query: { + query: 'test query', + dataset: { + dataSource: { + id: 'test-id', + meta: { + name: 'test-name', + sessionId: 'test-session', + }, + }, + }, + }, + format: 'jdbc', + lang: 'sql', + }, + }; + }); + + describe('describeQuery', () => { + it('should handle request with complete dataset information', async () => { + mockClient.mockResolvedValue({ result: 'success' }); + + const result = await facet.describeQuery(mockContext, mockRequest); + + expect(result).toEqual({ success: true, data: { result: 'success' } }); + expect(mockClient).toHaveBeenCalledWith('test-endpoint', { + body: { + query: 'test query', + datasource: 'test-name', + sessionId: 'test-session', + lang: 'sql', + }, + }); + }); + + it('should handle request with missing dataSource', async () => { + mockRequest.body.query.dataset.dataSource = undefined; + mockClient.mockResolvedValue({ result: 'success' }); + + const result = await facet.describeQuery(mockContext, mockRequest); + + expect(result).toEqual({ success: true, data: { result: 'success' } }); + expect(mockClient).toHaveBeenCalledWith('test-endpoint', { + body: { + query: 'test query', + lang: 'sql', + }, + }); + }); + + it('should handle request with missing dataset', async () => { + mockRequest.body.query.dataset = undefined; + mockClient.mockResolvedValue({ result: 'success' }); + + const result = await facet.describeQuery(mockContext, mockRequest); + + expect(result).toEqual({ success: true, data: { result: 'success' } }); + expect(mockClient).toHaveBeenCalledWith('test-endpoint', { + body: { + query: 'test query', + lang: 'sql', + }, + }); + }); + + it('should handle errors', async () => { + const error = new Error('Test error'); + mockClient.mockRejectedValue(error); + + const result = await facet.describeQuery(mockContext, mockRequest); + + expect(result).toEqual({ success: false, data: error }); + expect(mockLogger.error).toHaveBeenCalledWith( + 'Facet fetch: test-endpoint: Error: Test error' + ); + }); + }); +}); diff --git a/src/plugins/query_enhancements/server/utils/facet.ts b/src/plugins/query_enhancements/server/utils/facet.ts index f86a07b5432c..0b6dd52407cd 100644 --- a/src/plugins/query_enhancements/server/utils/facet.ts +++ b/src/plugins/query_enhancements/server/utils/facet.ts @@ -38,8 +38,8 @@ export class Facet { ): Promise => { try { const query: Query = request.body.query; - const { dataSource } = query.dataset!; - const { meta } = dataSource!; + const dataSource = query.dataset?.dataSource; + const meta = dataSource?.meta; const { format, lang } = request.body; const params = { body: { From 0c047dc390cdfd4a4e01b9d3f01ef2b830ce9a03 Mon Sep 17 00:00:00 2001 From: Sean Li Date: Thu, 5 Sep 2024 13:36:36 -0700 Subject: [PATCH 11/12] [Fix] Show Alias Fields in Discover Tab (#7930) * Add field search option Signed-off-by: Suchit Sahoo * initial implementation for new setting Signed-off-by: Sean Li * Changeset file for PR #7930 created/updated * update naming Signed-off-by: Sean Li * removing page reload Signed-off-by: Sean Li * updating names Signed-off-by: Sean Li --------- Signed-off-by: Suchit Sahoo Signed-off-by: Sean Li Co-authored-by: Suchit Sahoo Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/7930.yml | 2 ++ src/plugins/data/common/constants.ts | 1 + .../common/search/search_source/search_source.ts | 7 +++++-- src/plugins/data/server/ui_settings.ts | 13 +++++++++++++ 4 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 changelogs/fragments/7930.yml diff --git a/changelogs/fragments/7930.yml b/changelogs/fragments/7930.yml new file mode 100644 index 000000000000..ce3a0c638262 --- /dev/null +++ b/changelogs/fragments/7930.yml @@ -0,0 +1,2 @@ +fix: +- Show alias fields in Discover tab ([#7930](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7930)) \ No newline at end of file diff --git a/src/plugins/data/common/constants.ts b/src/plugins/data/common/constants.ts index 289d56d4d064..199391630b25 100644 --- a/src/plugins/data/common/constants.ts +++ b/src/plugins/data/common/constants.ts @@ -86,6 +86,7 @@ export const UI_SETTINGS = { COURIER_BATCH_SEARCHES: 'courier:batchSearches', SEARCH_INCLUDE_FROZEN: 'search:includeFrozen', SEARCH_TIMEOUT: 'search:timeout', + SEARCH_INCLUDE_ALL_FIELDS: 'search:includeAllFields', HISTOGRAM_BAR_TARGET: 'histogram:barTarget', HISTOGRAM_MAX_BARS: 'histogram:maxBars', HISTORY_LIMIT: 'history:limit', diff --git a/src/plugins/data/common/search/search_source/search_source.ts b/src/plugins/data/common/search/search_source/search_source.ts index c8eb99b038a7..6a2eee0162b5 100644 --- a/src/plugins/data/common/search/search_source/search_source.ts +++ b/src/plugins/data/common/search/search_source/search_source.ts @@ -131,6 +131,7 @@ export const searchSourceRequiredUiSettings = [ UI_SETTINGS.SEARCH_INCLUDE_FROZEN, UI_SETTINGS.SORT_OPTIONS, UI_SETTINGS.QUERY_DATAFRAME_HYDRATION_STRATEGY, + UI_SETTINGS.SEARCH_INCLUDE_ALL_FIELDS, ]; export interface SearchSourceDependencies extends FetchHandlers { @@ -586,6 +587,7 @@ export class SearchSource { flatten() { const searchRequest = this.mergeProps(); + const { getConfig } = this.dependencies; searchRequest.body = searchRequest.body || {}; const { body, index, fields, query, filters, highlightAll } = searchRequest; @@ -595,6 +597,9 @@ export class SearchSource { body.stored_fields = computedFields.storedFields; body.script_fields = body.script_fields || {}; + if (getConfig(UI_SETTINGS.SEARCH_INCLUDE_ALL_FIELDS)) { + body.fields = ['*']; + } extend(body.script_fields, computedFields.scriptFields); const defaultDocValueFields = computedFields.docvalueFields @@ -606,8 +611,6 @@ export class SearchSource { body._source = index.getSourceFiltering(); } - const { getConfig } = this.dependencies; - if (body._source) { // exclude source fields for this index pattern specified by the user const filter = fieldWildcardFilter(body._source.excludes, getConfig(UI_SETTINGS.META_FIELDS)); diff --git a/src/plugins/data/server/ui_settings.ts b/src/plugins/data/server/ui_settings.ts index 79cf50a900b9..4cee0a9894dd 100644 --- a/src/plugins/data/server/ui_settings.ts +++ b/src/plugins/data/server/ui_settings.ts @@ -762,5 +762,18 @@ export function getUiSettings(): Record> { }), schema: schema.arrayOf(schema.string()), }, + [UI_SETTINGS.SEARCH_INCLUDE_ALL_FIELDS]: { + name: i18n.translate('data.advancedSettings.searchIncludeAllFieldsTitle', { + defaultMessage: 'Include all fields in search request', + }), + value: false, + description: i18n.translate('data.advancedSettings.searchIncludeAllFieldsText', { + defaultMessage: ` + Experimental: + Adds the "fields": ["*"] property to search request body`, + }), + schema: schema.boolean(), + category: ['search'], + }, }; } From 7b83b5737e655a316e5a47de7b7c1e5532c67b7e Mon Sep 17 00:00:00 2001 From: Lu Yu Date: Thu, 5 Sep 2024 13:50:14 -0700 Subject: [PATCH 12/12] add Huy as maintainer (#8025) * add Huy as maintainer Signed-off-by: Lu Yu * Changeset file for PR #8025 created/updated --------- Signed-off-by: Lu Yu Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- .github/CODEOWNERS | 2 +- MAINTAINERS.md | 1 + changelogs/fragments/8025.yml | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/8025.yml diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 891c858aa1a5..a0069ba271d0 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1 +1 @@ -* @ananzh @kavilla @AMoo-Miki @ashwin-pc @joshuarrrr @abbyhu2000 @zengyan-amazon @zhongnansu @manasvinibs @ZilongX @Flyingliuhub @curq @bandinib-amzn @SuZhou-Joe @ruanyl @BionIT @xinruiba @zhyuanqi @mengweieric @LDrago27 @virajsanghvi @sejli @joshuali925 +* @ananzh @kavilla @AMoo-Miki @ashwin-pc @joshuarrrr @abbyhu2000 @zengyan-amazon @zhongnansu @manasvinibs @ZilongX @Flyingliuhub @curq @bandinib-amzn @SuZhou-Joe @ruanyl @BionIT @xinruiba @zhyuanqi @mengweieric @LDrago27 @virajsanghvi @sejli @joshuali925 @huyaboo diff --git a/MAINTAINERS.md b/MAINTAINERS.md index 321d9ab02794..9f964d82a66c 100644 --- a/MAINTAINERS.md +++ b/MAINTAINERS.md @@ -29,6 +29,7 @@ This document contains a list of maintainers in this repo. See [opensearch-proje | Viraj Sanghvi | [virajsanghvi](https://github.com/virajsanghvi) | Amazon | | Sean Li | [sejli](https://github.com/sejli) | Amazon | | Joshua Li | [joshuali925](https://github.com/joshuali925) | Amazon | +| Huy Nguyen | [huyaboo](https://github.com/huyaboo) | Amazon | ## Emeritus diff --git a/changelogs/fragments/8025.yml b/changelogs/fragments/8025.yml new file mode 100644 index 000000000000..af483ae30ec3 --- /dev/null +++ b/changelogs/fragments/8025.yml @@ -0,0 +1,2 @@ +doc: +- Add Huy as maintainer ([#8025](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8025)) \ No newline at end of file