-
Notifications
You must be signed in to change notification settings - Fork 916
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] Use data source filter function before rendering #6175
Conversation
Signed-off-by: Lu Yu <nluyu@amazon.com>
Signed-off-by: Lu Yu <nluyu@amazon.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6175 +/- ##
==========================================
- Coverage 67.25% 67.25% -0.01%
==========================================
Files 3345 3345
Lines 64840 64835 -5
Branches 10435 10432 -3
==========================================
- Hits 43606 43602 -4
Misses 18690 18690
+ Partials 2544 2543 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Lu Yu <nluyu@amazon.com>
LGTM, picked up latest main branch and now rerun all checks |
@@ -119,6 +99,16 @@ export class DataSourceSelector extends React.Component< | |||
this.props.placeholderText === undefined | |||
? 'Select a data source' | |||
: this.props.placeholderText; | |||
|
|||
const dataSources = this.props.dataSourceFilter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have UT to cover this changes? Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. Approved but I'm wondering if we need UT for this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we use the same unit test for the filtering which was added when the datasource filter was introduced https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/data_source_management/public/components/data_source_selector/data_source_selector.test.tsx#L137
#6175) * use filter function before rendering Signed-off-by: Lu Yu <nluyu@amazon.com> * add change log Signed-off-by: Lu Yu <nluyu@amazon.com> --------- Signed-off-by: Lu Yu <nluyu@amazon.com> Co-authored-by: ZilongX <99905560+ZilongX@users.noreply.github.com> (cherry picked from commit 8975381) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md
#6175) (#6265) * use filter function before rendering Signed-off-by: Lu Yu <nluyu@amazon.com> * add change log Signed-off-by: Lu Yu <nluyu@amazon.com> --------- Signed-off-by: Lu Yu <nluyu@amazon.com> Co-authored-by: ZilongX <99905560+ZilongX@users.noreply.github.com> (cherry picked from commit 8975381) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
This change will store all data sources fetched during initial load, then apply the datasource filter function before rendering so that it can filter based on changes in the filter function.
Issues Resolved
Fixes #6174
Screenshot
filterbeforerender.mp4
Testing the changes
The following steps were performed in the recording:
Check List
yarn test:jest
yarn test:jest_integration