Skip to content

[Lens] Code cleanup#221248

Merged
markov00 merged 6 commits intoelastic:mainfrom
markov00:2025_05_22-code_cleanup
May 28, 2025
Merged

[Lens] Code cleanup#221248
markov00 merged 6 commits intoelastic:mainfrom
markov00:2025_05_22-code_cleanup

Conversation

@markov00
Copy link
Contributor

@markov00 markov00 commented May 22, 2025

Summary

This PR removes a piece of code that doesn't have effects in the overall logic.
The PR adds also a minimal cleanup on a return type to simplify the logic a bit.

Comment on lines -711 to -733
// use current frame api and patch apis for changed datasource layers
if (
visualizableState.keptLayerIds &&
visualizableState.datasourceId &&
visualizableState.datasourceState
) {
const datasource = datasources[visualizableState.datasourceId];
const datasourceState = visualizableState.datasourceState;
const updatedLayerApis: DatasourceLayers = pick(
frame.datasourceLayers,
visualizableState.keptLayerIds
);
const changedLayers = datasource.getLayers(visualizableState.datasourceState);
changedLayers.forEach((layerId) => {
if (updatedLayerApis[layerId]) {
updatedLayerApis[layerId] = datasource.getPublicAPI({
layerId,
state: datasourceState,
indexPatterns: frame.dataViews.indexPatterns,
});
}
});
}
Copy link
Contributor Author

@markov00 markov00 May 22, 2025

Choose a reason for hiding this comment

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

none of these variables are used outside that if and I hope the getPublicAPI or the getLayers are pure functions that doesn't modifies any external state.

@elastic elastic deleted a comment from elasticmachine May 23, 2025
@markov00 markov00 marked this pull request as ready for review May 26, 2025 07:24
@markov00 markov00 requested a review from a team as a code owner May 26, 2025 07:24
@markov00 markov00 added Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// release_note:skip Skip the PR/issue when compiling release notes backport:version Backport to applied version labels v9.1.0 v8.19.0 labels May 26, 2025
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

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.6MB 1.6MB -323.0B

History

Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

I've added a side effect as it was probably intended for the removed block and could no see any change locally.
I'll approve. In case of issues let's revert and add the side effect.

@markov00 markov00 merged commit a066428 into elastic:main May 28, 2025
11 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.19

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 28, 2025
## Summary

This PR removes a piece of code that doesn't have effects in the overall
logic.
The PR adds also a minimal cleanup on a return type to simplify the
logic a bit.

(cherry picked from commit a066428)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.19

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 May 28, 2025
# Backport

This will backport the following commits from `main` to `8.19`:
- [[Lens] Code cleanup
(#221248)](#221248)

<!--- Backport version: 9.6.6 -->

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

<!--BACKPORT [{"author":{"name":"Marco
Vettorello","email":"marco.vettorello@elastic.co"},"sourceCommit":{"committedDate":"2025-05-28T12:16:58Z","message":"[Lens]
Code cleanup (#221248)\n\n## Summary\n\nThis PR removes a piece of code
that doesn't have effects in the overall\nlogic.\nThe PR adds also a
minimal cleanup on a return type to simplify the\nlogic a
bit.","sha":"a066428708b7c4bb185c3c5eb1080faae8e2aff3","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Visualizations","release_note:skip","backport:version","v9.1.0","v8.19.0"],"title":"[Lens]
Code
cleanup","number":221248,"url":"https://github.com/elastic/kibana/pull/221248","mergeCommit":{"message":"[Lens]
Code cleanup (#221248)\n\n## Summary\n\nThis PR removes a piece of code
that doesn't have effects in the overall\nlogic.\nThe PR adds also a
minimal cleanup on a return type to simplify the\nlogic a
bit.","sha":"a066428708b7c4bb185c3c5eb1080faae8e2aff3"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/221248","number":221248,"mergeCommit":{"message":"[Lens]
Code cleanup (#221248)\n\n## Summary\n\nThis PR removes a piece of code
that doesn't have effects in the overall\nlogic.\nThe PR adds also a
minimal cleanup on a return type to simplify the\nlogic a
bit.","sha":"a066428708b7c4bb185c3c5eb1080faae8e2aff3"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request May 29, 2025
## Summary

This PR removes a piece of code that doesn't have effects in the overall
logic.
The PR adds also a minimal cleanup on a return type to simplify the
logic a bit.
dej611 added a commit that referenced this pull request May 30, 2025
## Summary

Fixes #178571 

This PR reverts #221248 adding the missing part on it to make the bug
found in #178571 disappear.
In fact there was just a single line missing who was meant to keep the
suggestion frame version up to date with the latest workspace state.


![lens_suggestion_bug_fix](https://github.com/user-attachments/assets/ff59e4d6-cfac-4801-857a-a79b1c98762d)

I've tried to add a unit test but it was so hard to actually follow the
code underneath, so I settled to add a FTR, which led to few more
extras:

* subtype selector now has test ids
* Lens page now has few more utility helpers for layer cloning and sub
type selector

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
dej611 added a commit to dej611/kibana that referenced this pull request Jun 3, 2025
…21901)

## Summary

Fixes elastic#178571

This PR reverts elastic#221248 adding the missing part on it to make the bug
found in elastic#178571 disappear.
In fact there was just a single line missing who was meant to keep the
suggestion frame version up to date with the latest workspace state.

![lens_suggestion_bug_fix](https://github.com/user-attachments/assets/ff59e4d6-cfac-4801-857a-a79b1c98762d)

I've tried to add a unit test but it was so hard to actually follow the
code underneath, so I settled to add a FTR, which led to few more
extras:

* subtype selector now has test ids
* Lens page now has few more utility helpers for layer cloning and sub
type selector

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 0bcd7ff)

# Conflicts:
#	x-pack/test/functional/page_objects/lens_page.ts
dej611 added a commit that referenced this pull request Jun 3, 2025
…21901) (#222380)

# Backport

This will backport the following commits from `main` to `8.19`:
- [[Lens] Keep suggestions up to date with the main workspace
(#221901)](#221901)

<!--- Backport version: 10.0.0 -->

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

<!--BACKPORT [{"author":{"name":"Marco
Liberati","email":"dej611@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-05-30T14:14:32Z","message":"[Lens]
Keep suggestions up to date with the main workspace (#221901)\n\n##
Summary\n\nFixes #178571 \n\nThis PR reverts #221248 adding the missing
part on it to make the bug\nfound in #178571 disappear.\nIn fact there
was just a single line missing who was meant to keep the\nsuggestion
frame version up to date with the latest workspace
state.\n\n\n![lens_suggestion_bug_fix](https://github.com/user-attachments/assets/ff59e4d6-cfac-4801-857a-a79b1c98762d)\n\nI've
tried to add a unit test but it was so hard to actually follow the\ncode
underneath, so I settled to add a FTR, which led to few
more\nextras:\n\n* subtype selector now has test ids\n* Lens page now
has few more utility helpers for layer cloning and sub\ntype
selector\n\n### Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"0bcd7ffcddb706da6407337ad3e3e85e0713fb13","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Visualizations","release_note:skip","Feature:Lens","backport
missing","backport:version","v9.1.0","v8.19.0"],"title":"[Lens] Keep
suggestions up to date with the main
workspace","number":221901,"url":"https://github.com/elastic/kibana/pull/221901","mergeCommit":{"message":"[Lens]
Keep suggestions up to date with the main workspace (#221901)\n\n##
Summary\n\nFixes #178571 \n\nThis PR reverts #221248 adding the missing
part on it to make the bug\nfound in #178571 disappear.\nIn fact there
was just a single line missing who was meant to keep the\nsuggestion
frame version up to date with the latest workspace
state.\n\n\n![lens_suggestion_bug_fix](https://github.com/user-attachments/assets/ff59e4d6-cfac-4801-857a-a79b1c98762d)\n\nI've
tried to add a unit test but it was so hard to actually follow the\ncode
underneath, so I settled to add a FTR, which led to few
more\nextras:\n\n* subtype selector now has test ids\n* Lens page now
has few more utility helpers for layer cloning and sub\ntype
selector\n\n### Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"0bcd7ffcddb706da6407337ad3e3e85e0713fb13"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/221901","number":221901,"mergeCommit":{"message":"[Lens]
Keep suggestions up to date with the main workspace (#221901)\n\n##
Summary\n\nFixes #178571 \n\nThis PR reverts #221248 adding the missing
part on it to make the bug\nfound in #178571 disappear.\nIn fact there
was just a single line missing who was meant to keep the\nsuggestion
frame version up to date with the latest workspace
state.\n\n\n![lens_suggestion_bug_fix](https://github.com/user-attachments/assets/ff59e4d6-cfac-4801-857a-a79b1c98762d)\n\nI've
tried to add a unit test but it was so hard to actually follow the\ncode
underneath, so I settled to add a FTR, which led to few
more\nextras:\n\n* subtype selector now has test ids\n* Lens page now
has few more utility helpers for layer cloning and sub\ntype
selector\n\n### Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"0bcd7ffcddb706da6407337ad3e3e85e0713fb13"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
zacharyparikh pushed a commit to zacharyparikh/kibana that referenced this pull request Jun 4, 2025
## Summary

This PR removes a piece of code that doesn't have effects in the overall
logic.
The PR adds also a minimal cleanup on a return type to simplify the
logic a bit.
zacharyparikh pushed a commit to zacharyparikh/kibana that referenced this pull request Jun 4, 2025
…21901)

## Summary

Fixes elastic#178571 

This PR reverts elastic#221248 adding the missing part on it to make the bug
found in elastic#178571 disappear.
In fact there was just a single line missing who was meant to keep the
suggestion frame version up to date with the latest workspace state.


![lens_suggestion_bug_fix](https://github.com/user-attachments/assets/ff59e4d6-cfac-4801-857a-a79b1c98762d)

I've tried to add a unit test but it was so hard to actually follow the
code underneath, so I settled to add a FTR, which led to few more
extras:

* subtype selector now has test ids
* Lens page now has few more utility helpers for layer cloning and sub
type selector

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
nickpeihl pushed a commit to nickpeihl/kibana that referenced this pull request Jun 12, 2025
…21901)

## Summary

Fixes elastic#178571 

This PR reverts elastic#221248 adding the missing part on it to make the bug
found in elastic#178571 disappear.
In fact there was just a single line missing who was meant to keep the
suggestion frame version up to date with the latest workspace state.


![lens_suggestion_bug_fix](https://github.com/user-attachments/assets/ff59e4d6-cfac-4801-857a-a79b1c98762d)

I've tried to add a unit test but it was so hard to actually follow the
code underneath, so I settled to add a FTR, which led to few more
extras:

* subtype selector now has test ids
* Lens page now has few more utility helpers for layer cloning and sub
type selector

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants