[Dashboard] fix custom time ranges not applied to panel until global query context changes #155458
Conversation
… parent input subscription, remove session only check from should fetch logic
ThomThomson
added
Feature:Dashboard
|
@elasticmachine merge upstream |
…-ref HEAD~1..HEAD --fix'
…le parent updates
nreese
force-pushed
the
searchSession/cleanup
branch
from
b171d38 to
dbd7286
Compare
…-ref HEAD~1..HEAD --fix'
nreese
force-pushed
the
searchSession/cleanup
branch
from
2474fae to
88daae1
Compare
nreese
changed the title
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.test.ts
Outdated
Show resolved
Hide resolved
| if (!diffingFunction) { | ||
| return fastIsEqual(diffFunctionProps.currentValue, diffFunctionProps.lastValue); | ||
| } | ||
|
|
||
| if (diffingFunction?.prototype?.name === 'AsyncFunction') { | ||
| throw new Error( | ||
| `The function for key "${key}" is async, must use isKeyEqualAsync for asynchronous functions` | ||
| ); | ||
| } | ||
| return diffingFunction(propsAsNever); |
There was a problem hiding this comment.
minor nit: Hmm.... I wonder if we could make it clearer when we use isKeyEqualAsync versus isKeyEqual (synchronous) - either with a comment or perhaps by making the naming clearer? Like, isKeyEqualAsync -> unsavedChangesIsKeyEqual and isKeyEqual -> shouldRefreshIsKeyEqual... Not sure if that's too specific
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Data Discovery changes LGTM 👍
Is it expected that a custom time range is not applied when opening a saved search panel data in Discover via "Open in Discover" menu option? Nevermind, not related to this PR. Opened an issue #155852
kibanamachine
added
v8.8.0
backport:skip
Fixes #155409
#152516 incorrectly attempted to resolve #151221. #152516 updated shouldFetch$ to only check searchSessionId to determine if re-fetching is required. This logic did not work when custom time ranges are applied to panels since custom time ranges do not require new search session id yet the time range changed.
This PR reverts shouldFetch$ logic of only checking searchSessionId to determine if re-fetching is required.
Instead, this PR moves searchSessionId out of input and into dashboard instance state. That way,
inputupdates, such as query, do not trigger additionalinputupdates. The PR also updates seachSessionId logic from async to sync so that dashboard can update seachSessionId on input changes prior to child embeddables updating to parent input changes. This avoids the double fetch and allows children to only have a single input update on query state change.There was a functional test, panel_time_range, that should have caught the regression but that test had a bug in it. The assertion that the custom time range was applied looked like
expect(await testSubjects.exists('emptyPlaceholder'))which will never fail the test because the last part of the expect is missing. Instead, the statements should beexpect(await testSubjects.exists('emptyPlaceholder')).to.be(true). These updates to the functional test would have caught the regression (I verified this by making these changes on main and running the test. They do indeed fail).