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.

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

[Lens] Fix switchVisualizationType to use it without layerId #196295

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

mbondyra
Copy link
Contributor

@mbondyra mbondyra commented Oct 15, 2024

With PR #187475 we introduced a bug, affecting the AI assistant's suggestions API when switching between different chart types (e.g., from bar to line chart). This feature relies on the switchVisualizationType method, which was changed to require a layerId. However, the suggestions API does not provide layerId, causing the chart type to not switch as expected.

Solution:
The bug can be resolved by modifying the logic in the switchVisualizationType method. We changed:

const dataLayer = state.layers.find((l) => l.layerId === layerId);

to:

const dataLayer = state.layers.find((l) => l.layerId === layerId) ?? state.layers[0];

This ensures that the suggestions API falls back to the first layer if no specific layerId is provided.

@mbondyra mbondyra added Team:Visualizations Visualization editors, elastic-charts and infrastructure release_note:skip Skip the PR/issue when compiling release notes Feature:Lens backport:skip This commit does not require backporting labels Oct 15, 2024
@mbondyra mbondyra marked this pull request as ready for review October 15, 2024 12:25
@mbondyra mbondyra requested a review from a team as a code owner October 15, 2024 12:25
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbondyra thanx 🙇 LGTM!

@stratoula stratoula added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:skip This commit does not require backporting labels Oct 15, 2024
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #18 / DataTableColumnHeader should render a correct token

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 1.5MB 1.5MB +17.0B

History

  • 💔 Build #242606 failed 77abd20fe28cbfdc57c32fa3940a3b6ee8516108

@mbondyra mbondyra merged commit e476220 into elastic:main Oct 15, 2024
28 checks passed
@mbondyra mbondyra deleted the lens/fix_suggestions_api branch October 15, 2024 21:53
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11354960986

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 15, 2024
…#196295)

With [PR elastic#187475](https://github.com/elastic/kibana/pull/187475/files)
we introduced a bug, affecting the AI assistant's suggestions API when
switching between different chart types (e.g., from bar to line chart).
This feature relies on the switchVisualizationType method, which was
changed to require a `layerId`. However, the suggestions API does not
provide `layerId`, causing the chart type to not switch as expected.

Solution:
The bug can be resolved by modifying the logic in the
`switchVisualizationType` method. We changed:

```ts
const dataLayer = state.layers.find((l) => l.layerId === layerId);
```

to:

```ts
const dataLayer = state.layers.find((l) => l.layerId === layerId) ?? state.layers[0];
```

This ensures that the suggestions API falls back to the first layer if
no specific layerId is provided.

---------

Co-authored-by: Marco Vettorello <[email protected]>
(cherry picked from commit e476220)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 15, 2024
…196295) (#196448)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Lens] Fix switchVisualizationType to use it without layerId
(#196295)](#196295)

<!--- 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-15T21:53:00Z","message":"[Lens]
Fix switchVisualizationType to use it without layerId (#196295)\n\nWith
[PR #187475](https://github.com/elastic/kibana/pull/187475/files)\r\nwe
introduced a bug, affecting the AI assistant's suggestions API
when\r\nswitching between different chart types (e.g., from bar to line
chart).\r\nThis feature relies on the switchVisualizationType method,
which was\r\nchanged to require a `layerId`. However, the suggestions
API does not\r\nprovide `layerId`, causing the chart type to not switch
as expected.\r\n\r\nSolution:\r\nThe bug can be resolved by modifying
the logic in the\r\n`switchVisualizationType` method. We
changed:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) =>
l.layerId === layerId);\r\n```\r\n\r\nto:\r\n\r\n```ts\r\nconst
dataLayer = state.layers.find((l) => l.layerId === layerId) ??
state.layers[0];\r\n```\r\n\r\nThis ensures that the suggestions API
falls back to the first layer if\r\nno specific layerId is
provided.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello
<[email protected]>","sha":"e4762201fdd84f372c78bc2a159061e504b26e78","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Visualizations","release_note:skip","Feature:Lens","v9.0.0","backport:prev-minor"],"title":"[Lens]
Fix switchVisualizationType to use it without
layerId","number":196295,"url":"https://github.com/elastic/kibana/pull/196295","mergeCommit":{"message":"[Lens]
Fix switchVisualizationType to use it without layerId (#196295)\n\nWith
[PR #187475](https://github.com/elastic/kibana/pull/187475/files)\r\nwe
introduced a bug, affecting the AI assistant's suggestions API
when\r\nswitching between different chart types (e.g., from bar to line
chart).\r\nThis feature relies on the switchVisualizationType method,
which was\r\nchanged to require a `layerId`. However, the suggestions
API does not\r\nprovide `layerId`, causing the chart type to not switch
as expected.\r\n\r\nSolution:\r\nThe bug can be resolved by modifying
the logic in the\r\n`switchVisualizationType` method. We
changed:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) =>
l.layerId === layerId);\r\n```\r\n\r\nto:\r\n\r\n```ts\r\nconst
dataLayer = state.layers.find((l) => l.layerId === layerId) ??
state.layers[0];\r\n```\r\n\r\nThis ensures that the suggestions API
falls back to the first layer if\r\nno specific layerId is
provided.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello
<[email protected]>","sha":"e4762201fdd84f372c78bc2a159061e504b26e78"}},"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/196295","number":196295,"mergeCommit":{"message":"[Lens]
Fix switchVisualizationType to use it without layerId (#196295)\n\nWith
[PR #187475](https://github.com/elastic/kibana/pull/187475/files)\r\nwe
introduced a bug, affecting the AI assistant's suggestions API
when\r\nswitching between different chart types (e.g., from bar to line
chart).\r\nThis feature relies on the switchVisualizationType method,
which was\r\nchanged to require a `layerId`. However, the suggestions
API does not\r\nprovide `layerId`, causing the chart type to not switch
as expected.\r\n\r\nSolution:\r\nThe bug can be resolved by modifying
the logic in the\r\n`switchVisualizationType` method. We
changed:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) =>
l.layerId === layerId);\r\n```\r\n\r\nto:\r\n\r\n```ts\r\nconst
dataLayer = state.layers.find((l) => l.layerId === layerId) ??
state.layers[0];\r\n```\r\n\r\nThis ensures that the suggestions API
falls back to the first layer if\r\nno specific layerId is
provided.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello
<[email protected]>","sha":"e4762201fdd84f372c78bc2a159061e504b26e78"}}]}]
BACKPORT-->

Co-authored-by: Marta Bondyra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants