Skip to content

Commit

Permalink
[8.x] [Discover] Fix suggestions for ES|QL charts (#197583) (#197852)
Browse files Browse the repository at this point in the history 
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Discover] Fix suggestions for ES|QL charts
(#197583)](#197583)

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

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

<!--BACKPORT [{"author":{"name":"Marta
Bondyra","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-25T15:17:02Z","message":"[Discover]
Fix suggestions for ES|QL charts (#197583)\n\nFixes
https://github.com/elastic/kibana/issues/197342\r\n\r\nIn this PR
(#197101) I removed the\r\nlegacy
metric from being suggested in the suggestion panel, and replaced\r\nit
with the new metric visualization. To maintain the previous
behavior\r\nin Lens (suggesting a new metric in the same place as legacy
metric), we\r\nmade the score higher for the new metric. This positioned
it higher also\r\nin the Discover ESQL suggestions. This led to an issue
where one\r\nexpected suggestion didn’t appear because we only display
the top 6\r\nsuggestions by score and it got pushed out by
metric.\r\n\r\nAdditionally, I made a change here to only display the
metric without\r\nbucketed columns in the suggestion panel. I don't see
there's a lot of\r\nvalue in suggesting bucketed metric unless it's
something user chooses\r\nintentionally.\r\n\r\nShould be merged to 8.x
after
this:\r\nhttps://github.com//pull/197337","sha":"b3b85da80d0c9a8431f6a2f2e3c1bdf1448eb1a6","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Discover","Team:Visualizations","release_note:skip","v9.0.0","backport:prev-minor"],"title":"[Discover]
Fix suggestions for ES|QL
charts","number":197583,"url":"https://github.com/elastic/kibana/pull/197583","mergeCommit":{"message":"[Discover]
Fix suggestions for ES|QL charts (#197583)\n\nFixes
https://github.com/elastic/kibana/issues/197342\r\n\r\nIn this PR
(#197101) I removed the\r\nlegacy
metric from being suggested in the suggestion panel, and replaced\r\nit
with the new metric visualization. To maintain the previous
behavior\r\nin Lens (suggesting a new metric in the same place as legacy
metric), we\r\nmade the score higher for the new metric. This positioned
it higher also\r\nin the Discover ESQL suggestions. This led to an issue
where one\r\nexpected suggestion didn’t appear because we only display
the top 6\r\nsuggestions by score and it got pushed out by
metric.\r\n\r\nAdditionally, I made a change here to only display the
metric without\r\nbucketed columns in the suggestion panel. I don't see
there's a lot of\r\nvalue in suggesting bucketed metric unless it's
something user chooses\r\nintentionally.\r\n\r\nShould be merged to 8.x
after
this:\r\nhttps://github.com//pull/197337","sha":"b3b85da80d0c9a8431f6a2f2e3c1bdf1448eb1a6"}},"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/197583","number":197583,"mergeCommit":{"message":"[Discover]
Fix suggestions for ES|QL charts (#197583)\n\nFixes
https://github.com/elastic/kibana/issues/197342\r\n\r\nIn this PR
(#197101) I removed the\r\nlegacy
metric from being suggested in the suggestion panel, and replaced\r\nit
with the new metric visualization. To maintain the previous
behavior\r\nin Lens (suggesting a new metric in the same place as legacy
metric), we\r\nmade the score higher for the new metric. This positioned
it higher also\r\nin the Discover ESQL suggestions. This led to an issue
where one\r\nexpected suggestion didn’t appear because we only display
the top 6\r\nsuggestions by score and it got pushed out by
metric.\r\n\r\nAdditionally, I made a change here to only display the
metric without\r\nbucketed columns in the suggestion panel. I don't see
there's a lot of\r\nvalue in suggesting bucketed metric unless it's
something user chooses\r\nintentionally.\r\n\r\nShould be merged to 8.x
after
this:\r\nhttps://github.com//pull/197337","sha":"b3b85da80d0c9a8431f6a2f2e3c1bdf1448eb1a6"}}]}]
BACKPORT-->

Co-authored-by: Marta Bondyra <[email protected]>
  • Loading branch information
@kibanamachine @mbondyra
kibanamachine and mbondyra authored Oct 25, 2024
1 parent 4c22558 commit 3bf06e1
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
View file
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
10 changes: 5 additions & 5 deletions x-pack/plugins/lens/public/visualizations/metric/suggestions.test.ts
View file
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
4 changes: 1 addition & 3 deletions x-pack/plugins/lens/public/visualizations/metric/suggestions.ts
View file
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 3bf06e1

Please sign in to comment.