[Dashboard] Fix for controls selections causing multiple fetches#224761
Conversation
ThomThomson
added
Feature:Dashboard
| } | ||
| ) | ||
| ); | ||
| controlGroupSubscriptions.add(controlGroupFilters$.subscribe(() => panelsReload$.next())); |
There was a problem hiding this comment.
This is what was causing the double fetch. I think this is a holdover from when we stored unified search filters and control group filters differently, and needed some trigger to refresh the page when a control selection changed.
There was a problem hiding this comment.
You are correct, this panelsReload$.next() is carry over from DashboardContainer conversion. Now all filters are passed to children with filters$ and this is no longer needed.
Since you are moving this, controlGroupReload$ and panelsReload$ are no longer needed and can be combine into just reload$.
There was a problem hiding this comment.
I'm planning on doing a Controls X Dashboard architecture cleanup as my next major task, so I was planning on doing that there.
|
|
||
| if (apiPublishesReload(api)) { | ||
| observables.push(api.reload$); | ||
| observables.push(api.reload$.pipe(startWith(undefined))); |
There was a problem hiding this comment.
This is required because the combineLatest on line 67 wasn't getting called. This caused the Controls to not create a new search session ID, and therefore not work until after refresh was pressed once.
There was a problem hiding this comment.
Would you mind adding a test case to src/platform/plugins/shared/dashboard/public/dashboard_api/search_sessions/new_session.test.ts for this. Maybe just an api with { filters$, reload$ } and verify it fires when filters$ changes?
There was a problem hiding this comment.
Good call, will do!
There was a problem hiding this comment.
Added a test in a25410b and verified that it fails when the startWith(undefined) is missing.
|
@elasticmachine merge upstream |
|
/ci |
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
nreese
left a comment
nreese
left a comment
There was a problem hiding this comment.
LGTM - thanks for adding the test
code review only
…trolFetchPartialFix
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
History
|
|
Starting backport for target branches: 9.0 |
…stic#224761) Prevents Dashboard from firing two requests when a Control change is made. (cherry picked from commit b0d7180)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
|
How about backporting to 8.19? |
#224761) (#225517) # Backport This will backport the following commits from `main` to `9.0`: - [[Dashboard] Fix for controls selections causing multiple fetches (#224761)](#224761) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Devon Thomson","email":"devon.thomson@elastic.co"},"sourceCommit":{"committedDate":"2025-06-26T16:34:16Z","message":"[Dashboard] Fix for controls selections causing multiple fetches (#224761)\n\nPrevents Dashboard from firing two requests when a Control change is made.","sha":"b0d7180ff889044622c6b1290787bde664057085","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Dashboard","release_note:fix","Feature:Input Control","Team:Presentation","loe:small","impact:high","backport:prev-minor","v9.1.0"],"title":"[Dashboard] Fix for controls selections causing multiple fetches","number":224761,"url":"https://github.com/elastic/kibana/pull/224761","mergeCommit":{"message":"[Dashboard] Fix for controls selections causing multiple fetches (#224761)\n\nPrevents Dashboard from firing two requests when a Control change is made.","sha":"b0d7180ff889044622c6b1290787bde664057085"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/224761","number":224761,"mergeCommit":{"message":"[Dashboard] Fix for controls selections causing multiple fetches (#224761)\n\nPrevents Dashboard from firing two requests when a Control change is made.","sha":"b0d7180ff889044622c6b1290787bde664057085"}}]}] BACKPORT--> Co-authored-by: Devon Thomson <devon.thomson@elastic.co>
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…es (#224761) (#225537) # Backport This will backport the following commits from `main` to `8.19`: - [[Dashboard] Fix for controls selections causing multiple fetches (#224761)](#224761) <!--- Backport version: 10.0.1 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Devon Thomson","email":"devon.thomson@elastic.co"},"sourceCommit":{"committedDate":"2025-06-26T16:34:16Z","message":"[Dashboard] Fix for controls selections causing multiple fetches (#224761)\n\nPrevents Dashboard from firing two requests when a Control change is made.","sha":"b0d7180ff889044622c6b1290787bde664057085","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Dashboard","release_note:fix","Feature:Input Control","Team:Presentation","loe:small","impact:high","backport:prev-minor","v9.1.0","v8.19.0","v9.0.4"],"title":"[Dashboard] Fix for controls selections causing multiple fetches","number":224761,"url":"https://github.com/elastic/kibana/pull/224761","mergeCommit":{"message":"[Dashboard] Fix for controls selections causing multiple fetches (#224761)\n\nPrevents Dashboard from firing two requests when a Control change is made.","sha":"b0d7180ff889044622c6b1290787bde664057085"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/224761","number":224761,"mergeCommit":{"message":"[Dashboard] Fix for controls selections causing multiple fetches (#224761)\n\nPrevents Dashboard from firing two requests when a Control change is made.","sha":"b0d7180ff889044622c6b1290787bde664057085"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.0","label":"v9.0.4","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/225517","number":225517,"state":"MERGED","mergeCommit":{"sha":"e6ed7bcdcac2234c4be611670fa2e73d9c2e786d","message":"[9.0] [Dashboard] Fix for controls selections causing multiple fetches (#224761) (#225517)\n\n# Backport\n\nThis will backport the following commits from `main` to `9.0`:\n- [[Dashboard] Fix for controls selections causing multiple fetches\n(#224761)](https://github.com/elastic/kibana/pull/224761)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sorenlouv/backport)\n\n\n\nCo-authored-by: Devon Thomson <devon.thomson@elastic.co>"}}]}] BACKPORT--> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
4 participants
Summary
Fixes #224626
This PR prevents Dashboard from firing two requests
How to test
add
tap(() => console.log('fetch!'))at the end of the pipe on line 150 ofsrc/platform/packages/shared/presentation/presentation_publishing/interfaces/fetch/fetch.tsand verify that it fires only once when Controls selections are madeKnown issue
This PR does not fix the Controls double-fetch that happens when loading a dashboard with a controls selection already made. This will not be fixed until #219637 is fixed.