Skip to content

Commit

Permalink
[Discover] Fix suggestions for ES|QL charts (elastic#197583)
Browse files Browse the repository at this point in the history
Fixes elastic#197342

In this PR (elastic#197101) I removed the
legacy metric from being suggested in the suggestion panel, and replaced
it with the new metric visualization. To maintain the previous behavior
in Lens (suggesting a new metric in the same place as legacy metric), we
made the score higher for the new metric. This positioned it higher also
in the Discover ESQL suggestions. This led to an issue where one
expected suggestion didn’t appear because we only display the top 6
suggestions by score and it got pushed out by metric.

Additionally, I made a change here to only display the metric without
bucketed columns in the suggestion panel. I don't see there's a lot of
value in suggesting bucketed metric unless it's something user chooses
intentionally.

Should be merged to 8.x after this:
elastic#197337
  • Loading branch information
mbondyra authored Oct 25, 2024
1 parent 0ff9a8a commit b3b85da
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 14 deletions.
11 changes: 5 additions & 6 deletions test/functional/apps/discover/group3/_lens_vis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
return seriesType;
}

// Failing: See https://github.com/elastic/kibana/issues/197342
describe.skip('discover lens vis', function () {
describe('discover lens vis', function () {
before(async () => {
await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']);
await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional');
Expand Down Expand Up @@ -616,8 +615,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(await getCurrentVisTitle()).to.be('Pie');
await testSubjects.existOrFail('partitionVisChart');

await discover.chooseLensSuggestion('barVerticalStacked');
await changeVisShape('Line');
await discover.chooseLensSuggestion('waffle');
await changeVisShape('Treemap');

await testSubjects.existOrFail('unsavedChangesBadge');
await discover.saveUnsavedChanges();
Expand All @@ -626,8 +625,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await discover.waitUntilSearchingHasFinished();

await testSubjects.missingOrFail('unsavedChangesBadge');
expect(await getCurrentVisTitle()).to.be('Line');
await testSubjects.existOrFail('xyVisChart');
expect(await getCurrentVisTitle()).to.be('Treemap');
await testSubjects.existOrFail('partitionVisChart');
});

it('should close lens flyout on revert changes', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ describe('metric suggestions', () => {
breakdownByAccessor: bucketColumn.columnId,
},
title: 'Metric',
hide: false,
hide: true,
previewIcon: IconChartMetric,
score: 0.51,
},
Expand Down Expand Up @@ -221,7 +221,7 @@ describe('metric suggestions', () => {
breakdownByAccessor: bucketColumn.columnId,
},
title: 'Metric',
hide: false,
hide: true,
previewIcon: IconChartMetric,
score: 0.51,
},
Expand Down Expand Up @@ -294,7 +294,7 @@ describe('metric suggestions', () => {
breakdownByAccessor: bucketColumn.columnId,
},
title: 'Metric',
hide: false,
hide: true,
previewIcon: IconChartMetric,
score: 0.52,
},
Expand Down Expand Up @@ -326,7 +326,7 @@ describe('metric suggestions', () => {
breakdownByAccessor: bucketColumn.columnId,
},
title: 'Metric',
hide: false,
hide: true,
previewIcon: IconChartMetric,
score: 0.52,
},
Expand Down Expand Up @@ -357,7 +357,7 @@ describe('metric suggestions', () => {
breakdownByAccessor: bucketColumn.columnId,
},
title: 'Metric',
hide: false,
hide: true,
previewIcon: IconChartMetric,
score: 0.52,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ export const getSuggestions: Visualization<MetricVisualizationState>['getSuggest

const bucketedColumns = table.columns.filter(({ operation }) => operation.isBucketed);

const hasInterval = bucketedColumns.some(({ operation }) => operation.scale === 'interval');

const unsupportedColumns = table.columns.filter(
({ operation }) => !supportedDataTypes.has(operation.dataType) && !operation.isBucketed
);
Expand Down Expand Up @@ -64,7 +62,7 @@ export const getSuggestions: Visualization<MetricVisualizationState>['getSuggest
title: metricColumns[0]?.operation.label || metricLabel,
previewIcon: IconChartMetric,
score: 0.5,
hide: hasInterval,
hide: !!bucketedColumns.length,
};

const accessorMappings: Pick<MetricVisualizationState, 'metricAccessor' | 'breakdownByAccessor'> =
Expand Down

0 comments on commit b3b85da

Please sign in to comment.