OpenSearch-Dashboards icon indicating copy to clipboard operation
OpenSearch-Dashboards copied to clipboard

[Multiple Datasource]DataSourceView add switch to default data source button

Open yujin-emma opened this issue 10 months ago • 10 comments

Description

Add switch to default data source when pass in invalid data source id for DataSourceView We expect to see When pass in invalid data source id

default data source is available no default data source
hide local cluster show switch button and provide the switch to default data source functionality on both popover and toast no switch button display either on popover or toast
not hide local cluster show switch button and provide the switch to default data source functionality on both popover and toast no switch button display either on popover or toast

Issues Resolved

Screenshot

Testing the changes

Happy case: when pass in valid id without label, the DataSourceView can display the correct label happy-case

Happy case but the result got filter out, we treat it as invalid id error:

 activeOption: [{ id: '829c0ee0-065f-11ef-ad27-4377edb9ad8f' }],
          dataSourceFilter: (ds) => {
            return ds.id !== '829c0ee0-065f-11ef-ad27-4377edb9ad8f'
          },

happy-case-but-filterout

Overall Test Result:

When pass in invalid data source id

default data source is available no default data source
hide local cluster hide-lc-default-show-button Screenshot 2024-04-23 at 14 07 48Screenshot 2024-04-23 at 14 07 41
not hide local cluster both-show-button Screenshot 2024-04-23 at 14 07 41Screenshot 2024-04-23 at 14 07 48

test input:

<DataSourceMenu
        setMenuMountPoint={setActionMenu}
        componentType={'DataSourceView'}
        componentConfig={{
          notifications,
          savedObjects: savedObjects.client,
          fullWidth: false,
          activeOption: [{ id: 'invalid-id' }],
          dataSourceFilter: (ds) => {
            return true;
          },
          onSelectedDataSources: (ds) => {
            setSelectedDataSources(ds);
          },
        }}
      />
    );

1. When hide local cluster, and there is no default data source

Screenshot 2024-04-23 at 14 07 48 Screenshot 2024-04-23 at 14 07 41

2. When hide local cluster, and there is default data source

hide-lc-default-show-button

3. Test mobile view

Screenshot 2024-04-23 at 11 43 21

4. Test iphone SE View

Screenshot 2024-04-23 at 12 00 46

5. When not hide local cluster:

5.1. When no default data source - both the popover and toast should not display the button to switch the default data source
Screenshot 2024-04-23 at 14 07 41 Screenshot 2024-04-23 at 14 07 48
5.2. When there is default data source - both the popover and toast should display the switch button to switch to the default data source

both-show-button

6. If only pass in valid data source id, the DataSourceView will render the correct data source title, if click the header button, the popover will show up and only checked with the activeOption, there is no error toast, no switch to default button

Test input from examples:

<DataSourceMenu
        setMenuMountPoint={setActionMenu}
        componentType={'DataSourceView'}
        componentConfig={{
          notifications,
          savedObjects: savedObjects.client,
          fullWidth: false,
          activeOption: [{ id: '98652370-01b8-11ef-9df2-d1bcc041cdef' }], // this is basically the test2 data source id
          dataSourceFilter: (ds) => {
            return true;
          },
          onSelectedDataSources: (ds) => {
            setSelectedDataSources(ds);
          },
        }}
      />

Screenshot 2024-04-23 at 14 30 36

Changelog

  • skip

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

yujin-emma avatar Apr 23 '24 06:04 yujin-emma

Codecov Report

Attention: Patch coverage is 38.37209% with 53 lines in your changes missing coverage. Please review.

Project coverage is 67.37%. Comparing base (32fbe18) to head (060370f). Report is 10 commits behind head on main.

Files Patch % Lines
...c/components/data_source_view/data_source_view.tsx 36.20% 36 Missing and 1 partial :warning:
.../data_source_management/public/components/utils.ts 7.69% 12 Missing :warning:
...onents/data_source_view/data_source_view_error.tsx 66.66% 3 Missing :warning:
.../components/toast_button/switch_default_button.tsx 50.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6610      +/-   ##
==========================================
- Coverage   67.43%   67.37%   -0.07%     
==========================================
  Files        3444     3446       +2     
  Lines       67847    67906      +59     
  Branches    11035    11047      +12     
==========================================
- Hits        45755    45750       -5     
- Misses      19426    19491      +65     
+ Partials     2666     2665       -1     
Flag Coverage Δ
Linux_1 33.08% <ø> (ø)
Linux_2 55.12% <ø> (ø)
Linux_3 ?
Linux_4 34.78% <5.95%> (-0.04%) :arrow_down:
Windows_1 33.11% <ø> (-0.03%) :arrow_down:
Windows_2 55.09% <ø> (ø)
Windows_3 45.21% <38.37%> (-0.06%) :arrow_down:
Windows_4 34.78% <5.95%> (-0.04%) :arrow_down:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 23 '24 06:04 codecov[bot]

5 ci group failed somehow, re-running now

ZilongX avatar Apr 23 '24 20:04 ZilongX

When hide local cluster, and there is no default data source

What's the input for the data source component? Also can we show the popover?

BionIT avatar Apr 23 '24 20:04 BionIT

When hide local cluster, and there is default data source

The toast message should change too and can you double check the requirement?

BionIT avatar Apr 23 '24 20:04 BionIT

Can we add test for when invalid datasource id gets passed in and there is default data source, we should the right popover with toast and be able to switch to use default data source?

BionIT avatar Apr 23 '24 20:04 BionIT

Can we add test for when invalid datasource id gets passed in and there is default data source, we should the right popover with toast and be able to switch to use default data source?

Test 5.2 cover this case

yujin-emma avatar Apr 23 '24 21:04 yujin-emma

When hide local cluster, and there is default data source

The toast message should change too and can you double check the requirement?

Test 2 cover this

yujin-emma avatar Apr 23 '24 21:04 yujin-emma

When hide local cluster, and there is no default data source

What's the input for the data source component? Also can we show the popover?

Thanks for pointing it out, already updated in the descriptions with test set up

yujin-emma avatar Apr 23 '24 21:04 yujin-emma

@yujin-emma Great job, the PR description is very detailed

zhongnansu avatar Apr 26 '24 21:04 zhongnansu

Failed codecov, do we have enough tests? Follow https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6610/checks?check_run_id=24317184661 to add tests

BionIT avatar Apr 26 '24 22:04 BionIT

@yujin-emma Is this still being targeted for 2.16.

LDrago27 avatar Jul 23 '24 23:07 LDrago27