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] Enables ad-hoc dataviews #138732

Merged
merged 15 commits into from
Aug 29, 2022
Merged

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Aug 12, 2022

Summary

Related to #137809

Implements the ad-hoc dataviews in Lens.

Ad hoc data views persist as long as the user doesn't navigate away from Lens (they are shown in the existing data view pickers in unified search and layer panel together with the regular data views without differentiation). When saving, the data views are saved along with the saved object.

When changing an ad-hoc data view (adding, editing or deleting a field), it gets recreated with a new id (the old one is deleted from the cache) to prevent id conflicts with other ad hoc data views in other Lens visualizations. To do this, a new "renameIndexPattern" callback has been introduced on visualization and datasource side.

The ad-hoc-ness of data views is transparent to the datasource (and the visualization once query based annotations are introduced) - this means the references are still extracted as before. The frame checks whether there are any references to ad hoc data views and stores them separately in an internalReferences array.

lens1

App services changes

Filters that are referencing local data views can not be "injected" as usual - this PR adjusts the logic of leaving the index alone in this situation instead of setting it to undefined.

Split out of this PR

  • Open in Discover (is disabled) can be added once ad hoc data view feature is merged in Discover
  • Open geo field in maps (won't show geo fields)can be added once ad hoc data view feature is merged in Maps

@@ -1,93 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
Copy link
Contributor Author

@stratoula stratoula Aug 12, 2022

Choose a reason for hiding this comment

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

ℹ️ I am deleting this file because it is not used anymore in Lens. This must be a leftover from the phase 1 (or from a merge conflict). I can create a separate PR for the removal but as I am dealing here with the dataview picker lists I think I can also remove it here :)

@stratoula stratoula mentioned this pull request Aug 12, 2022
7 tasks
@flash1293 flash1293 marked this pull request as ready for review August 17, 2022 10:47
@flash1293 flash1293 requested review from a team as code owners August 17, 2022 10:47
@flash1293 flash1293 marked this pull request as draft August 17, 2022 10:47
@flash1293
Copy link
Contributor

Hit the wrong button, sorry. This is quite ready, but not 100%.

@flash1293 flash1293 marked this pull request as ready for review August 17, 2022 15:53
@flash1293 flash1293 added release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens backport:skip This commit does not require backporting labels Aug 17, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

@flash1293
Copy link
Contributor

OK, it's ready now

@stratoula
Copy link
Contributor Author

@flash1293 thanx so much for continuing my work here! This looks fantastic! I really appreciate it ❤️

Copy link
Contributor Author

@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.

This works fine! Code LGTM! I tested it locally to see how it works inside the editor and when embedded on the dashboard and it works as expected.
Joe did the majority of the work here but as I was involved we would appreciate another review here! (I cant approve as this is from my fork)

@flash1293
Copy link
Contributor

The test subject is now called lnsMetric_primaryMetricDimensionPanel

const testSubjects = getService('testSubjects');
const filterBar = getService('filterBar');
const retry = getService('retry');

const getMetricTiles = () =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ As we are using the new metric viz on the tests, I moved these functions to the lens page to be reusable.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
lens 522 527 +5

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.2MB 1.2MB +4.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 429.6KB 429.6KB +12.0B
unifiedSearch 46.8KB 46.9KB +140.0B
total +152.0B
Unknown metric groups

API count

id before after diff
lens 608 613 +5
unifiedSearch 108 109 +1
total +6

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

The code looks great, superb job! Tested and i couldn't find any problems with it.

I talked with Stratoula about how we persist dataviews and she explained it nicely (we persist it even if we create it but then remove it from usage so the user doesn't have to recreate it when removing a layer). Approved!

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

Response Ops changes LGTM!

@stratoula stratoula merged commit 255f24d into elastic:main Aug 29, 2022
Mpdreamz pushed a commit to Mpdreamz/kibana that referenced this pull request Sep 6, 2022
* [Lens] Enables ad-hoc dataviews

* Fix bug in legacy field existence logic

* lift ad hoc state to frame level and rename data view on incompatible change

* fix some tests

* migrate embedded ad hoc data views

* fix tests and inject/extract ad hoc data view references

* fix bugs and add functional tests

* fix unit tests

* do not show geo fields for ad hoc data views

* Fix functional test

* Refactor to use the new metric viz on the functional tests

Co-authored-by: Joe Reuter <johannes.reuter@elastic.co>
kibanamachine added a commit that referenced this pull request Dec 8, 2022
# Backport

This will backport the following commits from `main` to `8.6`:
- [Adds the VisEditor docs for 8.6
(#146471)](#146471)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Kaarina
Tungseth","email":"kaarina.tungseth@elastic.co"},"sourceCommit":{"committedDate":"2022-12-08T20:18:03Z","message":"Adds
the VisEditor docs for 8.6 (#146471)\n\n## Summary\r\n\r\nAdds the 8.6
for the following:\r\n\r\n- #140878, #143946 and
#142187\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/tsvb.html#edit-visualizations-in-lens)\r\n\r\n-
#142936, #142561, #143820, and
#142838\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/add-aggregation-based-visualization-panels.html#edit-agg-based-visualizations-in-lens)\r\n\r\n-
#138732\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#change-the-fields)\r\n\r\n-
#141626 and #141615\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#add-annotations)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Stratoula Kalafateli
<stratoula1@gmail.com>","sha":"f918a3745be3badff9cf05950db61d7f47877961","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Docs","release_note:skip","v8.6.0","v8.7.0"],"number":146471,"url":"#146471
the VisEditor docs for 8.6 (#146471)\n\n## Summary\r\n\r\nAdds the 8.6
for the following:\r\n\r\n- #140878, #143946 and
#142187\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/tsvb.html#edit-visualizations-in-lens)\r\n\r\n-
#142936, #142561, #143820, and
#142838\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/add-aggregation-based-visualization-panels.html#edit-agg-based-visualizations-in-lens)\r\n\r\n-
#138732\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#change-the-fields)\r\n\r\n-
#141626 and #141615\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#add-annotations)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Stratoula Kalafateli
<stratoula1@gmail.com>","sha":"f918a3745be3badff9cf05950db61d7f47877961"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"#146471
the VisEditor docs for 8.6 (#146471)\n\n## Summary\r\n\r\nAdds the 8.6
for the following:\r\n\r\n- #140878, #143946 and
#142187\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/tsvb.html#edit-visualizations-in-lens)\r\n\r\n-
#142936, #142561, #143820, and
#142838\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/add-aggregation-based-visualization-panels.html#edit-agg-based-visualizations-in-lens)\r\n\r\n-
#138732\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#change-the-fields)\r\n\r\n-
#141626 and #141615\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#add-annotations)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Stratoula Kalafateli
<stratoula1@gmail.com>","sha":"f918a3745be3badff9cf05950db61d7f47877961"}}]}]
BACKPORT-->

Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Lens release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants