Skip to content

Commit

Permalink
[8.x] [Discover] Remove redundant data fetching when hiding/showing t…
Browse files Browse the repository at this point in the history 
…he histogram/chart (#206389) (#207051)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Discover] Remove redundant data fetching when hiding/showing the
histogram/chart
(#206389)](#206389)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Matthias
Wilhelm","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-17T10:49:37Z","message":"[Discover]
Remove redundant data fetching when hiding/showing the histogram/chart
(#206389)\n\nSince the timerange in Discover of the main request is
stable we don't need to trigger a main fetch for all data when the
histogram/chart is being hidden/displayed, unless it's necessary to get
the data (e.g. when the histogram/chart was hiden when a discover
session was being
loaded)","sha":"a04274723ef64182df96d37d134cc645e20571ee","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","Feature:Discover","performance","v9.0.0","Team:DataDiscovery","backport:prev-minor"],"title":"[Discover]
Remove redundant data fetching when hiding/showing the
histogram/chart","number":206389,"url":"https://github.com/elastic/kibana/pull/206389","mergeCommit":{"message":"[Discover]
Remove redundant data fetching when hiding/showing the histogram/chart
(#206389)\n\nSince the timerange in Discover of the main request is
stable we don't need to trigger a main fetch for all data when the
histogram/chart is being hidden/displayed, unless it's necessary to get
the data (e.g. when the histogram/chart was hiden when a discover
session was being
loaded)","sha":"a04274723ef64182df96d37d134cc645e20571ee"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/206389","number":206389,"mergeCommit":{"message":"[Discover]
Remove redundant data fetching when hiding/showing the histogram/chart
(#206389)\n\nSince the timerange in Discover of the main request is
stable we don't need to trigger a main fetch for all data when the
histogram/chart is being hidden/displayed, unless it's necessary to get
the data (e.g. when the histogram/chart was hiden when a discover
session was being
loaded)","sha":"a04274723ef64182df96d37d134cc645e20571ee"}}]}]
BACKPORT-->

Co-authored-by: Matthias Wilhelm <[email protected]>
  • Loading branch information
@kibanamachine @kertal
kibanamachine and kertal authored Jan 17, 2025
1 parent 815fc06 commit 06063bb
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 25 deletions.
5 changes: 1 addition & 4 deletions ...s/shared/discover/public/application/main/state_management/utils/build_state_subscribe.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,9 @@ export const buildStateSubscribe =
}
}

const { hideChart, interval, breakdownField, sampleSize, sort, dataSource } = prevState;
const { interval, breakdownField, sampleSize, sort, dataSource } = prevState;
// Cast to boolean to avoid false positives when comparing
// undefined and false, which would trigger a refetch
const chartDisplayChanged = Boolean(nextState.hideChart) !== Boolean(hideChart);
const chartIntervalChanged = nextState.interval !== interval && !isEsqlMode;
const breakdownFieldChanged = nextState.breakdownField !== breakdownField;
const sampleSizeChanged = nextState.sampleSize !== sampleSize;
Expand Down Expand Up @@ -137,7 +136,6 @@ export const buildStateSubscribe =
}

if (
chartDisplayChanged ||
chartIntervalChanged ||
breakdownFieldChanged ||
sampleSizeChanged ||
Expand All @@ -146,7 +144,6 @@ export const buildStateSubscribe =
queryChanged
) {
const logData = {
chartDisplayChanged: logEntry(chartDisplayChanged, hideChart, nextState.hideChart),
chartIntervalChanged: logEntry(chartIntervalChanged, interval, nextState.interval),
breakdownFieldChanged: logEntry(
breakdownFieldChanged,
Expand Down
40 changes: 22 additions & 18 deletions test/functional/apps/discover/group3/_request_counts.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,28 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
});
});

it(`should send no requests (documents + chart) when toggling the chart visibility`, async () => {
await expectSearches(type, 0, async () => {
// hide chart
await discover.toggleChartVisibility();
// show chart
await discover.toggleChartVisibility();
});
});
it(`should send a request for chart data when toggling the chart visibility after a time range change`, async () => {
// hide chart
await discover.toggleChartVisibility();
await timePicker.setAbsoluteRange(
'Sep 21, 2015 @ 06:31:44.000',
'Sep 24, 2015 @ 00:00:00.000'
);
await waitForLoadingToFinish();
await expectSearches(type, 1, async () => {
// show chart, we expect a request for the chart data, since the time range changed
await discover.toggleChartVisibility();
});
});

it(`should send ${savedSearchesRequests} requests for saved search changes`, async () => {
await setQuery(query1);
await queryBar.clickQuerySubmitButton();
Expand Down Expand Up @@ -213,15 +235,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
setQuery: (query) => queryBar.setQuery(query),
});

it(`should send no more than 2 requests (documents + chart) when toggling the chart visibility`, async () => {
await expectSearches(type, 2, async () => {
await discover.toggleChartVisibility();
});
await expectSearches(type, 2, async () => {
await discover.toggleChartVisibility();
});
});

it('should send no more than 2 requests (documents + chart) when adding a filter', async () => {
await expectSearches(type, 2, async () => {
await filterBar.addFilter({
Expand Down Expand Up @@ -286,15 +299,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
setQuery: (query) => monacoEditor.setCodeEditorValue(query),
expectedRequests: 2,
});

it(`should send requests (documents + chart) when toggling the chart visibility`, async () => {
await expectSearches(type, 1, async () => {
await discover.toggleChartVisibility();
});
await expectSearches(type, 3, async () => {
await discover.toggleChartVisibility();
});
});
});
});
}
19 changes: 16 additions & 3 deletions x-pack/test_serverless/functional/test_suites/common/discover/group3/_request_counts.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,24 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
});
});

it('should send no more than 2 requests (documents + chart) when toggling the chart visibility', async () => {
await expectSearches(type, 2, async () => {
it(`should send no requests (documents + chart) when toggling the chart visibility`, async () => {
await expectSearches(type, 0, async () => {
// hide chart
await PageObjects.discover.toggleChartVisibility();
// show chart
await PageObjects.discover.toggleChartVisibility();
});
await expectSearches(type, 2, async () => {
});
it(`should send a request for chart data when toggling the chart visibility after a time range change`, async () => {
// hide chart
await PageObjects.discover.toggleChartVisibility();
await PageObjects.timePicker.setAbsoluteRange(
'Sep 21, 2015 @ 06:31:44.000',
'Sep 24, 2015 @ 00:00:00.000'
);
await waitForLoadingToFinish();
await expectSearches(type, 1, async () => {
// show chart, we expect a request for the chart data, since the time range changed
await PageObjects.discover.toggleChartVisibility();
});
});
Expand Down

0 comments on commit 06063bb

Please sign in to comment.