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.

Sign up for GitHub

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

[8.x] [Discover] Remove redundant data fetching when hiding/showing the histogram/chart (#206389) #207051

Merged
merged 1 commit into from
Jan 17, 2025
+39 −25
Merged

[8.x] [Discover] Remove redundant data fetching when hiding/showing the histogram/chart (#206389) #207051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions ...s/shared/discover/public/application/main/state_management/utils/build_state_subscribe.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading