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

[UnifiedSearch] Allow editing ad-hoc data views without permissions #142723

Merged
merged 7 commits into from Oct 6, 2022

Conversation

flash1293
Copy link
Contributor

Part of #141806

Allows to add/remove/edit fields for ad hoc data views in Discover and Lens, as well as "managing" ad hoc data views (changing pattern, default time field) in Lens.

@flash1293 flash1293 added release_note:enhancement Feature:Discover Discover Application Team:Visualizations Visualization editors, elastic-charts and infrastructure Team:AppServicesSv Feature:Lens Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) labels Oct 5, 2022
@flash1293 flash1293 marked this pull request as ready for review October 5, 2022 18:07
@flash1293 flash1293 requested review from a team as code owners October 5, 2022 18:07
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServicesSv)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@stratoula
Copy link
Contributor

@flash1293 one question! I have created the kibana_sample_data_ecom* ad-hoc dataview (from the picker)

I assume that the default time field has been selected for this, which must be the order_date. When I click the Manage button, the flyout opens with the timestamp field dropdown as disabled (and the order_date not selected). If I want to change the time field I have to make a change in the pattern. Is this what we want?

image

@flash1293
Copy link
Contributor Author

@stratoula this is a bug in the data view editor at the moment - on opening it doesn't compute matching indices right, that's why it's disabled. If you change the pattern, then change it back to the original one it becomes enabled. @mattkime is working on fixing this on another PR.

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.

oh nice! Got it, thanx Joe! In that case LGTM!

Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Discover changes LGTM 👍

@jughosta
Copy link
Contributor

jughosta commented Oct 6, 2022

There is one more place in Discover where we allow field editing: column dropdown
https://github.com/elastic/kibana/blob/main/src/plugins/discover/public/components/discover_grid/build_edit_field_button.tsx#L32

Screenshot 2022-10-06 at 09 54 16

Would be great to update it too.

@flash1293
Copy link
Contributor Author

Good catch @jughosta , added the updated condition there as well.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

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
dataViewEditor 31.1KB 31.2KB +59.0B
discover 479.0KB 479.0KB +43.0B
lens 1.3MB 1.3MB +34.0B
total +136.0B

Page load bundle

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

id before after diff
unifiedSearch 48.6KB 48.6KB +69.0B

History

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

@flash1293 flash1293 enabled auto-merge (squash) October 6, 2022 09:57
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

code lgtm

defaultMessage: 'Manage this data view',
})}
</EuiContextMenuItem>,
onEditDataView || dataViewEditor.userPermissions.editDataView() ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

readabiily nit: move this outside this .push and add a separate if check

@flash1293 flash1293 merged commit 0a01a92 into elastic:main Oct 6, 2022
@kibanamachine kibanamachine added v8.6.0 backport:skip This commit does not require backporting labels Oct 6, 2022
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 11, 2022
…lastic#142723)

* allow editing ad-hoc data views without permissions

* [CI] Auto-commit changed files from 'node scripts/build_plugin_list_docs'

* fxi tests

* fix test

* allow field editing from discover table

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 14, 2022
…lastic#142723)

* allow editing ad-hoc data views without permissions

* [CI] Auto-commit changed files from 'node scripts/build_plugin_list_docs'

* fxi tests

* fix test

* allow field editing from discover table

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
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:Discover Discover Application Feature:Lens release_note:enhancement Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants