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

[Dashboard] Fix bugs on removing pinned global filters on dashboard page #5143

Merged

Conversation

abbyhu2000
Copy link
Member

Description

This PR fixes the error removing pinned global filters.

Previously removing pinned global filters needs a refresh for it to be effective on the dashboard because it does not get updated immediately from global states to the dashboard container. Previously the dashboard container only listens to the app filters, this PR adds a listener on the global filter for the dashboard container.

Issues Resolved

resolves #4767

Screenshot

Screen.Recording.2023-09-27.at.10.34.53.PM.mov

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Merging #5143 (fe92f26) into main (588efcd) will decrease coverage by 0.03%.
Report is 1 commits behind head on main.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #5143      +/-   ##
==========================================
- Coverage   66.74%   66.72%   -0.03%     
==========================================
  Files        3278     3278              
  Lines       62985    62988       +3     
  Branches    10029    10029              
==========================================
- Hits        42040    42027      -13     
- Misses      18475    18490      +15     
- Partials     2470     2471       +1     
Flag Coverage Δ
Linux_1 35.31% <0.00%> (-0.01%) ⬇️
Linux_2 55.24% <ø> (ø)
Linux_3 43.75% <0.00%> (-0.02%) ⬇️
Linux_4 35.42% <0.00%> (-0.01%) ⬇️
Windows_2 ?
Windows_4 35.42% <0.00%> (?)

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

Files Coverage Δ
.../application/utils/use/use_dashboard_app_state.tsx 50.00% <0.00%> (-2.95%) ⬇️

... and 5 files with indirect coverage changes

@abbyhu2000 abbyhu2000 added the Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry label Sep 28, 2023
@@ -193,6 +193,17 @@ export const useDashboardAppAndGlobalState = ({

subscriptions.add(stopSyncingFromTimeFilters);

const stopSyncingFromGlobalFilters = filterManager.getUpdates$().subscribe(() => {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldnt we unsubscribe from this when exiting the hook?

Copy link
Member Author

Choose a reason for hiding this comment

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

i did unsubscribe by subscriptions.add(stopSyncingFromGlobalFilters); and subscriptions.unsubscribe();

@joshuarrrr
Copy link
Member

@abbyhu2000 any plans to also add a regression test for this? Maybe in the functional tests?

@joshuarrrr joshuarrrr merged commit ab925eb into opensearch-project:main Oct 3, 2023
54 of 55 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 3, 2023
Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
Co-authored-by: Miki <miki@amazon.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
(cherry picked from commit ab925eb)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
joshuarrrr added a commit that referenced this pull request Oct 3, 2023
(cherry picked from commit ab925eb)

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.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: Miki <miki@amazon.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
Leo7Deng pushed a commit to Leo7Deng/OpenSearch-Dashboards that referenced this pull request Oct 4, 2023
Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
Co-authored-by: Miki <miki@amazon.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: Leo Deng <leo7deng@gmail.com>
Leo7Deng pushed a commit to Leo7Deng/OpenSearch-Dashboards that referenced this pull request Oct 4, 2023
Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
Co-authored-by: Miki <miki@amazon.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: Leo Deng <leo7deng@gmail.com>
willie-hung pushed a commit to willie-hung/OpenSearch-Dashboards that referenced this pull request Oct 5, 2023
Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
Co-authored-by: Miki <miki@amazon.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: Willie Hung <willie880201044@gmail.com>
SuZhou-Joe pushed a commit to SuZhou-Joe/OpenSearch-Dashboards that referenced this pull request Oct 7, 2023
Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
Co-authored-by: Miki <miki@amazon.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x dashboards de-angular de-angularize work Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry v2.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Pinned filters do not work on visualisations/dashboards and do not update added or removed
5 participants