[Workspace] Integrate workspace front end with data connection type saved object #8013
Conversation
raintygao
force-pushed
the
connection
branch
from
ef45821
to
64d6c94
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8013 +/- ##
==========================================
+ Coverage 64.09% 64.11% +0.01%
==========================================
Files 3744 3744
Lines 88822 88842 +20
Branches 13841 13847 +6
==========================================
+ Hits 56931 56958 +27
+ Misses 31277 31270 -7
Partials 614 614
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -158,3 +160,9 @@ export enum AssociationDataSourceModalMode { | |||
| DirectQueryConnections = 'direction-query-connections', | |||
| } | |||
| export const USE_CASE_PREFIX = 'use-case-'; | |||
|
|
|||
| // Workspace will handle both data source and data connection type saved object. | |||
| export const WORKSPACE_DATA_SOURCE_AND_CONNECTION_OBJECT_TYPES = [ | |||
There was a problem hiding this comment.
any thought about using the data structure type?
example:
There was a problem hiding this comment.
Looks good, will take a look on whether we could use this.
There was a problem hiding this comment.
- Why isn't type in https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/data/common/datasets/types.ts#L41
data-sourcewhich is a type for data source object? - Is there an example for
data-connectiontype object? - Im thinking whether data structure type would be a little complex for workspace, because workspace may don't need dataset, column, table related info.
src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx
Outdated
Show resolved
Hide resolved
| @@ -31,6 +31,9 @@ import { HttpStart, NotificationsStart, SavedObjectsStart } from '../../../../.. | |||
| import { AssociationDataSourceModalMode } from '../../../common/constants'; | |||
| import { Logos } from '../../../../../core/common'; | |||
| import { DirectQueryConnectionIcon } from '../workspace_form'; | |||
| import { DataConnectionIcon } from '../workspace_form'; | |||
There was a problem hiding this comment.
import this from line 33. also do we have multiple cases of defining icon throughout the app?
There was a problem hiding this comment.
Yes, this is still pending as I'm finding a better way to maintain different state between data-source, query connection, data-connection.
| @@ -31,6 +31,9 @@ import { HttpStart, NotificationsStart, SavedObjectsStart } from '../../../../.. | |||
| import { AssociationDataSourceModalMode } from '../../../common/constants'; | |||
| import { Logos } from '../../../../../core/common'; | |||
| import { DirectQueryConnectionIcon } from '../workspace_form'; | |||
| import { DataConnectionIcon } from '../workspace_form'; | |||
|
|
|||
| import { DATA_CONNECTION_SAVED_OBJECT_TYPE } from '../../../../data_source/common/data_connections'; | |||
|
|
|||
| const ConnectionIcon = ({ | |||
There was a problem hiding this comment.
imo this could be cleaned up with using the data structures https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/data/common/datasets/types.ts#L75.
where we have the information about the icon and type
There was a problem hiding this comment.
Thanks, will take a look.
There was a problem hiding this comment.
I checked the interface of this data structure and haven't seen any related integration with data-connection type object. I suppose this is still in progress?
|
Server side changes are extracted in (#8200) in order to start testing as soon as possible. |
| return [ | ||
| const dataConnections = dataSources | ||
| .filter((ds) => ds.type === DATA_CONNECTION_SAVED_OBJECT_TYPE) | ||
| .map((ds) => ({ ...ds, name: ds.id })); |
There was a problem hiding this comment.
The type of data source connection will be displayed in the data source connection table. There will be a type filter at the top right of the data source connection table. There is a type property in the definition of data connection saved object. Will it been displayed in the data source connection table? If yes, i'm prefer using DataSourceConnectionType.DataConnection to indicate the new data-connection;
raintygao
force-pushed
the
connection
branch
from
4af08ef
to
7c1ca5d
Compare
raintygao
force-pushed
the
connection
branch
from
6301de0
to
0587aea
Compare
raintygao
changed the title
raintygao
requested review from
ananzh,
AMoo-Miki,
ashwin-pc,
joshuarrrr,
abbyhu2000,
zengyan-amazon,
zhongnansu,
manasvinibs and
ZilongX
as code owners
| @@ -9,7 +9,6 @@ import { i18n } from '@osd/i18n'; | |||
| import { DataSourceConnection, DataSourceConnectionType } from '../../../common/types'; | |||
| import { AssociationDataSourceModalMode } from '../../../common/constants'; | |||
| import { DataSourceConnectionTable } from '../workspace_form'; | |||
|
|
|||
There was a problem hiding this comment.
Nit: this empty line should be kept
| const workspaces = source.workspaces ?? []; | ||
| const auth = source.get('auth'); | ||
| const description = source.get('description'); | ||
| const dataSourceEngineType = source.get('dataSourceEngineType'); | ||
| const type = source.type; | ||
| // This is a field only for detail type of data connection in order not to mix saved object type. |
There was a problem hiding this comment.
Nit:
| // This is a field only for detail type of data connection in order not to mix saved object type. | |
| // This is a field only for detail type of data connection in order not to mix with saved object type. |
| }; | ||
| }); | ||
|
|
||
| export const convertDataSourcesToDataConnections = ( |
There was a problem hiding this comment.
Nit: maybe can consider to combine convertDataSourcesToOpenSearchConnections and convertDataSourcesToDataConnections
| ...fulfillRelatedConnections(openSearchConnections, directQueryConnections), | ||
| ...directQueryConnections, | ||
| ].sort((a, b) => a.name.localeCompare(b.name)); | ||
| ...dataConnections, | ||
| ].sort((a, b) => a?.name?.localeCompare(b?.name)); |
There was a problem hiding this comment.
It doesn't seem optional chain is needed here since the type definition shows name is mandatory.
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Co-authored-by: SuZhou-Joe <suzhou@amazon.com> Signed-off-by: Tianyu Gao <tygao@amazon.com>
raintygao
force-pushed
the
connection
branch
from
fe658c5
to
c87b2d8
Compare
…aved object (#8013) * feat: integrate workspace with data connections Signed-off-by: tygao <tygao@amazon.com> * update workspace pages and hooks to integrate with data connection Signed-off-by: tygao <tygao@amazon.com> * Changeset file for PR #8013 created/updated * remove extra comments Signed-off-by: tygao <tygao@amazon.com> * update data source import Signed-off-by: tygao <tygao@amazon.com> * test: update tests Signed-off-by: tygao <tygao@amazon.com> * Changeset file for PR #8013 created/updated * unify connection type icon and connectionType Signed-off-by: tygao <tygao@amazon.com> * test: add tests Signed-off-by: tygao <tygao@amazon.com> * display text directly instead of link for data connection in table Signed-off-by: tygao <tygao@amazon.com> * Apply suggestions from code review Co-authored-by: SuZhou-Joe <suzhou@amazon.com> Signed-off-by: Tianyu Gao <tygao@amazon.com> * update Signed-off-by: tygao <tygao@amazon.com> --------- Signed-off-by: tygao <tygao@amazon.com> Signed-off-by: Tianyu Gao <tygao@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: SuZhou-Joe <suzhou@amazon.com> (cherry picked from commit c894584) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Integrate workspace with data connection type saved object in frontend
Screenshot
Testing the changes
Use APIs in #7925 to create data connection object, then goes to workspace create page to assign/unassign data connection objects.
Changelog
Check List
yarn test:jestyarn test:jest_integration