[Multiple Datasource] Add data source selection service to support storing and getting selected data source updates #6827

raintygao · 2024-05-24T03:32:36Z

Description

Add data source selection service in DSM.
Add dataSourceSelection instance as new props to each data source selector component and call selectDataSource when selecting data source.
Return this instance as setup dependency for other plugins.

Screenshot

case1.mp4

Set selected value and other plugins can get selected value.

case2.mp4

A component lifecycle from mount and select data source to unmount and remove.

Multiple selector components in page and data stored.

Issues Resolved

#6825
opensearch-project/dashboards-assistant#192

Changelog

feat: Add data source selection service to support storing and getting selected data source updates

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

codecov · 2024-05-24T03:48:32Z

Codecov Report

Attention: Patch coverage is 91.52542% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 67.43%. Comparing base (089a2de) to head (677bc9d).

Files	Patch %	Lines
...ents/data_source_selector/data_source_selector.tsx	75.00%	2 Missing ⚠️
...c/components/data_source_view/data_source_view.tsx	75.00%	1 Missing and 1 partial ⚠️
.../data_source_selectable/data_source_selectable.tsx	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6827      +/-   ##
==========================================
+ Coverage   67.38%   67.43%   +0.04%     
==========================================
  Files        3443     3444       +1     
  Lines       67797    67834      +37     
  Branches    11033    11031       -2     
==========================================
+ Hits        45688    45741      +53     
+ Misses      19440    19427      -13     
+ Partials     2669     2666       -3

Flag	Coverage Δ
Linux_1	`33.07% <ø> (ø)`
Linux_2	`55.12% <ø> (ø)`
Linux_3	`45.24% <91.52%> (?)`
Linux_4	`34.82% <25.42%> (+<0.01%)`	⬆️
Windows_1	`33.12% <ø> (+0.02%)`	⬆️
Windows_2	`55.09% <ø> (ø)`
Windows_3	`45.26% <91.52%> (+0.07%)`	⬆️
Windows_4	`34.82% <25.42%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: tygao <tygao@amazon.com>

zhyuanqi · 2024-05-24T17:51:16Z

Hi Tianyu. Could you help me to better understanding what we are trying to do here? I see that we have add the pops of DataSourceSelectable and states of ComponenId. What are we trying to achieve here? Thanks

seraphjiang · 2024-05-24T18:03:13Z

@raintygao looks like there are lots of UI change, would you add a few screenshot and video to help the review.

SuZhou-Joe · 2024-05-27T03:15:32Z

src/plugins/navigation/public/top_nav_menu/top_nav_menu.test.tsx

@@ -133,6 +136,7 @@ describe('TopNavMenu', () => {
            fullWidth: true,
            activeOption: [{ label: 'what', id: '1' }],
          },
+          dataSourceSelection,


Does it mean all the places that are using data source pickers need to pass the dataSourceSelection explicitly? I thought it is an internal state and plugins do not need to be aware of this context.

There is no need to pass this instance manually, because usually all components are created by functions provided by DSM and passed within the function. Except for some test cases when manually rendering components that need to pass.

raintygao · 2024-05-27T05:30:39Z

Hi Tianyu. Could you help me to better understanding what we are trying to do here? I see that we have add the pops of DataSourceSelectable and states of ComponenId. What are we trying to achieve here? Thanks

Hi Ella, unlike other plugins that need to select their own data sources, chatbot needs to get the data source selected by other applications from the OSD page. As these selector components don't store selected value and there isn't a centralized way in DSM to manage this, so this PR mainly does two things:

Add a new service to the DSM which supports storing and getting data source selected by each component.
After each selector component completes the selection, the value will be stored in the instance of the service, which can be got by other plugins.

raintygao · 2024-05-27T05:55:13Z

@raintygao looks like there are lots of UI change, would you add a few screenshot and video to help the review.

Sure. Although this PR makes updates in each UI component, it only adds the logic to store selected value in service after selecting the data source, and doesn't not make any changes to the UI view. The changes to assistant plugin are still in progress and depending on this PR, I have added a sample video to the PR content to demonstrate the purpose.

ruanyl · 2024-05-27T08:52:12Z

@raintygao could you update the PR description as well to includes the backgrounds of why you need to introduce these changes? I'm afraid many people don't have enough context to review the changes.

raintygao · 2024-05-27T11:43:30Z

@raintygao could you update the PR description as well to includes the backgrounds of why you need to introduce these changes? I'm afraid many people don't have enough context to review the changes.

Sure. Already shared background and design offline. Created a new issue in plugin repo opensearch-project/dashboards-assistant#192 and link to provide more context.

BionIT · 2024-05-28T04:43:15Z

src/plugins/data_source_management/public/components/data_source_menu/types.ts

@@ -40,6 +41,7 @@ export interface DataSourceMenuProps<T = any> {
  uiSettings?: IUiSettingsClient;
  application?: ApplicationStart;
  setMenuMountPoint?: (menuMount: MountPoint | undefined) => void;
+  dataSourceSelection: DataSourceSelection;


I think this should be part of componentConfig since not all components need to provide this as prop. Also, enforcing this prop as required would break all consumers, since it is provided by the DSM plugin, we can make it optional?

I prefer to let it out of componentConfig because this instance should be passed from DSM not other plugins. As this instance is shared and limited in DSM, so we can set it as strict required.

I initially thought that other plugins only can render DS selector components through the two functions ui.DataSourceSelector and ui.getDataSourceMenu (like this way https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/dev_tools/public/application.tsx#L118), so I only need to pass the dataSourceSelection instance to the component through this two functions of DSM.

Until I saw home plugin directly importing component class for rendering without using two functions above(https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/home/public/application/components/tutorial_directory.js#L236), in this way the dataSourceSelection would be missing in the component.

So I'm thinking that in order to safely share dataSourceSelection instance among all components and be compatible with all usage ways, what about using a getter in each component to get instance directly (like this way https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6827/files#diff-ec5b6ef64504177e50cad326dcc07ada2135faafe924d25b0f4df088bc7b4823R80), so that I no longer have to manually pass dataSourceSelection through ui.DataSourceSelector and ui.getDataSourceMenu.

Using getter sounds good

BionIT · 2024-05-28T04:50:17Z

src/plugins/data_source_management/public/service/data_source_selection_service.ts

+import { BehaviorSubject } from 'rxjs';
+import { DataSourceOption } from '../components/data_source_menu/types';
+
+export class DataSourceSelection {


Can the class name match the module name like DataSourceSelectionService?

Signed-off-by: tygao <tygao@amazon.com>

…oring and getting selected data source updates (#6827) * add data source selection service Signed-off-by: tygao <tygao@amazon.com> * export generateComponentId in util Signed-off-by: tygao <tygao@amazon.com> * update tests and type Signed-off-by: tygao <tygao@amazon.com> * update tests Signed-off-by: tygao <tygao@amazon.com> * update bind in components Signed-off-by: tygao <tygao@amazon.com> * Changeset file for PR #6827 created/updated * test: add tests for service Signed-off-by: tygao <tygao@amazon.com> * move dataSourceSelection out of componoent props Signed-off-by: tygao <tygao@amazon.com> * update service class name Signed-off-by: tygao <tygao@amazon.com> * use getter to replace dataSourceSelection props Signed-off-by: tygao <tygao@amazon.com> * add fallback for getter Signed-off-by: tygao <tygao@amazon.com> * test: add selection test case for component Signed-off-by: tygao <tygao@amazon.com> --------- Signed-off-by: tygao <tygao@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Lu Yu <nluyu@amazon.com> (cherry picked from commit e2c0df2) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…oring and getting selected data source updates (#6827) (#6875) * add data source selection service * export generateComponentId in util * update tests and type * update tests * update bind in components * Changeset file for PR #6827 created/updated * test: add tests for service * move dataSourceSelection out of componoent props * update service class name * use getter to replace dataSourceSelection props * add fallback for getter * test: add selection test case for component --------- (cherry picked from commit e2c0df2) Signed-off-by: tygao <tygao@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Lu Yu <nluyu@amazon.com>

…oring and getting selected data source updates (opensearch-project#6827) * add data source selection service Signed-off-by: tygao <tygao@amazon.com> * export generateComponentId in util Signed-off-by: tygao <tygao@amazon.com> * update tests and type Signed-off-by: tygao <tygao@amazon.com> * update tests Signed-off-by: tygao <tygao@amazon.com> * update bind in components Signed-off-by: tygao <tygao@amazon.com> * Changeset file for PR opensearch-project#6827 created/updated * test: add tests for service Signed-off-by: tygao <tygao@amazon.com> * move dataSourceSelection out of componoent props Signed-off-by: tygao <tygao@amazon.com> * update service class name Signed-off-by: tygao <tygao@amazon.com> * use getter to replace dataSourceSelection props Signed-off-by: tygao <tygao@amazon.com> * add fallback for getter Signed-off-by: tygao <tygao@amazon.com> * test: add selection test case for component Signed-off-by: tygao <tygao@amazon.com> --------- Signed-off-by: tygao <tygao@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Lu Yu <nluyu@amazon.com>

raintygao requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, curq, bandinib-amzn, SuZhou-Joe, ruanyl, BionIT, xinruiba, zhyuanqi and mengweieric as code owners May 24, 2024 03:32

github-actions bot added the valued-contributor label May 24, 2024

opensearch-changeset-bot bot added a commit to raintygao/OpenSearch-Dashboards that referenced this pull request May 24, 2024

Changeset file for PR opensearch-project#6827 created/updated

33c970c

raintygao and others added 8 commits May 24, 2024 19:29

add data source selection service

d60dc58

Signed-off-by: tygao <tygao@amazon.com>

export generateComponentId in util

99734b2

Signed-off-by: tygao <tygao@amazon.com>

update tests and type

4d6f407

Signed-off-by: tygao <tygao@amazon.com>

update tests

1ffca7f

Signed-off-by: tygao <tygao@amazon.com>

update bind in components

2ca9969

Signed-off-by: tygao <tygao@amazon.com>

Changeset file for PR opensearch-project#6827 created/updated

9aa5358

test: add tests for service

2112aea

Signed-off-by: tygao <tygao@amazon.com>

move dataSourceSelection out of componoent props

98b637d

Signed-off-by: tygao <tygao@amazon.com>

raintygao force-pushed the feat-selection branch from ee737c5 to 98b637d Compare May 24, 2024 12:41

SuZhou-Joe reviewed May 27, 2024

View reviewed changes

raintygao mentioned this pull request May 27, 2024

feat: update backend to support MDS opensearch-project/dashboards-assistant#190

Merged

4 tasks

BionIT reviewed May 28, 2024

View reviewed changes

update service class name

6b198ef

Signed-off-by: tygao <tygao@amazon.com>

yujin-emma assigned zhyuanqi May 28, 2024

raintygao and others added 4 commits May 29, 2024 11:27

use getter to replace dataSourceSelection props

939486b

Signed-off-by: tygao <tygao@amazon.com>

add fallback for getter

185367d

Signed-off-by: tygao <tygao@amazon.com>

test: add selection test case for component

6e78142

Signed-off-by: tygao <tygao@amazon.com>

Merge branch 'main' into feat-selection

4a29bb2

BionIT approved these changes May 29, 2024

View reviewed changes

Merge branch 'main' into feat-selection

677bc9d

zhyuanqi approved these changes May 30, 2024

View reviewed changes

BionIT merged commit e2c0df2 into opensearch-project:main May 31, 2024
67 checks passed

BionIT added backport 2.x v2.15.0 labels May 31, 2024

opensearch-trigger-bot bot mentioned this pull request May 31, 2024

[Backport 2.x] [Multiple Datasource] Add data source selection service to support storing and getting selected data source updates #6875

Merged

derek-ho mentioned this pull request May 31, 2024

[BUG] data_source_view crashes OSD when calling node scripts/build_opensearch_dashboards_platform_plugins.js #6878

Closed

BionIT mentioned this pull request May 31, 2024

[Bug] Remove unused import and property #6879

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Multiple Datasource] Add data source selection service to support storing and getting selected data source updates #6827

[Multiple Datasource] Add data source selection service to support storing and getting selected data source updates #6827

raintygao commented May 24, 2024 •

edited

Loading

codecov bot commented May 24, 2024 •

edited

Loading

zhyuanqi commented May 24, 2024 •

edited

Loading

seraphjiang commented May 24, 2024

SuZhou-Joe May 27, 2024

raintygao May 27, 2024

raintygao commented May 27, 2024

raintygao commented May 27, 2024 •

edited

Loading

ruanyl commented May 27, 2024 •

edited

Loading

raintygao commented May 27, 2024 •

edited

Loading

BionIT May 28, 2024

raintygao May 28, 2024 •

edited

Loading

BionIT May 29, 2024

BionIT May 28, 2024

[Multiple Datasource] Add data source selection service to support storing and getting selected data source updates #6827

[Multiple Datasource] Add data source selection service to support storing and getting selected data source updates #6827

Conversation

raintygao commented May 24, 2024 • edited Loading

Description

Screenshot

Issues Resolved

Changelog

Check List

codecov bot commented May 24, 2024 • edited Loading

Codecov Report

zhyuanqi commented May 24, 2024 • edited Loading

seraphjiang commented May 24, 2024

SuZhou-Joe May 27, 2024

Choose a reason for hiding this comment

raintygao May 27, 2024

Choose a reason for hiding this comment

raintygao commented May 27, 2024

raintygao commented May 27, 2024 • edited Loading

ruanyl commented May 27, 2024 • edited Loading

raintygao commented May 27, 2024 • edited Loading

BionIT May 28, 2024

Choose a reason for hiding this comment

raintygao May 28, 2024 • edited Loading

Choose a reason for hiding this comment

BionIT May 29, 2024

Choose a reason for hiding this comment

BionIT May 28, 2024

Choose a reason for hiding this comment

raintygao commented May 24, 2024 •

edited

Loading

codecov bot commented May 24, 2024 •

edited

Loading

zhyuanqi commented May 24, 2024 •

edited

Loading

raintygao commented May 27, 2024 •

edited

Loading

ruanyl commented May 27, 2024 •

edited

Loading

raintygao commented May 27, 2024 •

edited

Loading

raintygao May 28, 2024 •

edited

Loading