Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Multiple Datasource] Add new error database icon to handle error state for data source component #6570

Merged
merged 4 commits into from
Apr 22, 2024

Conversation

BionIT
Copy link
Collaborator

@BionIT BionIT commented Apr 21, 2024

Description

This change adds new error database icon, adds popover when the icon is clicked, and add button to refresh the page in both popover and toasts to handle network error state for data source component.

Issues Resolved

Screenshot

erroricon.mp4

Testing the changes

The following was performed in the recording

  1. force to throw error in multi select, go to example plugin and click multi selectable and see error toasts, click on refresh will refresh page, click on error icon will see popover, click on refresh will refresh page, click on manage will go to data source management
  2. force to throw error in selectable, go to example plugin and click selectable and see error toasts, click on refresh will refresh page, click on error icon will see popover, click on refresh will refresh page, click on manage will go to data source management
  3. force to throw error in readonly single data source, go to example plugin and click data source view and see error toasts, click on refresh will refresh page, click on error icon will see popover, click on refresh will refresh page, click on manage will go to data source management
  4. force to throw error in aggregated view, go to example plugin and click list all and see error toasts, click on refresh will refresh page, click on error icon will see popover, click on refresh will refresh page, click on manage will go to data source management
  5. force to throw error in aggregated view, go to example plugin and click list active and see error toasts, click on refresh will refresh page, click on error icon will see popover, click on refresh will refresh page, click on manage will go to data source management

Find screenshots of toast and popover below
Screenshot 2024-04-20 at 8 43 02 PM

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

*/
import React from 'react';

export const ErrorIcon = () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how's it look in dark mode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is dark mode
Screenshot 2024-04-22 at 9 46 43 AM

@kavilla
Copy link
Member

kavilla commented Apr 21, 2024

when do we expect the error state to be present? should it always have a default connect and fall back to that?

title: i18n.translate('dataSource.fetchDataSourceError', {
defaultMessage: 'Failed to fetch data sources',
}),
text: toMountPoint(getReloadButton()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we can show the error in more detail similar to how the Discover page shows the error response if clicked in the toast.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This current implementation based on requirement from UX. Since this is more of network call failure, user needs to reload the page in order to trigger another call.

@kavilla
Copy link
Member

kavilla commented Apr 21, 2024

totally unrelated to this PR. but every time the page refreshes is it getting data sources? if it is could we consider caching the data sources like how we cache index patterns. updating the data source on selection

@BionIT
Copy link
Collaborator Author

BionIT commented Apr 22, 2024

when do we expect the error state to be present? should it always have a default connect and fall back to that?

It only shows up when there is error when fetching data sources. Since the call failed, there will be no default data source. What we want user to do it to refresh the page

<EuiIcon type={'crossInCircleFilled'} color={'danger'} />
<EuiText color={'danger'}>Error</EuiText>
<EuiPopover
id={'dataSourceErrrorPopover'}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo dataSourceErrrorPopover, it should dataSourceErrorPopover

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Will fix!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed typo in new commit

Signed-off-by: Lu Yu <[email protected]>
Copy link

codecov bot commented Apr 22, 2024

Codecov Report

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

Project coverage is 45.22%. Comparing base (d2d410b) to head (9ede6f2).
Report is 16 commits behind head on main.

Files Patch % Lines
.../data_source_error_menu/data_source_error_menu.tsx 50.00% 4 Missing ⚠️
...lic/components/custom_database_icon/error_icon.tsx 50.00% 1 Missing ⚠️
...ce_aggregated_view/data_source_aggregated_view.tsx 0.00% 1 Missing ⚠️
...t/public/components/toast_button/reload_button.tsx 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #6570       +/-   ##
===========================================
+ Coverage   32.93%   45.22%   +12.28%     
===========================================
  Files        2260     1660      -600     
  Lines       45769    33701    -12068     
  Branches     7200     6409      -791     
===========================================
+ Hits        15075    15240      +165     
+ Misses      29984    17276    -12708     
- Partials      710     1185      +475     
Flag Coverage Δ
Linux_1 ?
Windows_3 45.22% <61.11%> (?)

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.

@BionIT BionIT merged commit b117fd3 into opensearch-project:main Apr 22, 2024
67 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 22, 2024
…te for data source component (#6570)

* add error icon and add popover

Signed-off-by: Lu Yu <[email protected]>

* fix tests

Signed-off-by: Lu Yu <[email protected]>

* fix typo

Signed-off-by: Lu Yu <[email protected]>

---------

Signed-off-by: Lu Yu <[email protected]>
(cherry picked from commit b117fd3)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
zhongnansu pushed a commit that referenced this pull request Apr 22, 2024
…te for data source component (#6570) (#6593)

* add error icon and add popover



* fix tests



* fix typo



---------


(cherry picked from commit b117fd3)

Signed-off-by: Lu Yu <[email protected]>
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>
LDrago27 pushed a commit to LDrago27/OpenSearch-Dashboards that referenced this pull request Jun 3, 2024
…te for data source component (opensearch-project#6570)

* add error icon and add popover

Signed-off-by: Lu Yu <[email protected]>

* fix tests

Signed-off-by: Lu Yu <[email protected]>

* fix typo

Signed-off-by: Lu Yu <[email protected]>

---------

Signed-off-by: Lu Yu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
all-star-contributor backport 2.x multiple datasource multiple datasource project Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry v2.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants