-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] Add description property and check duplicate title on save #68219
[Lens] Add description property and check duplicate title on save #68219
Conversation
Pinging @elastic/kibana-app (Team:KibanaApp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated app arch typings LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested in Safari. LGTM, although I would be happy to have another way of organizing saved visualizations.
@@ -34,10 +34,11 @@ export const getLensAliasConfig = (): VisTypeAlias => ({ | |||
searchFields: ['title^3'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is weird, because I can definitely search the description when I type a unique string, but I think this is supposed to be:
searchFields: ['title^3'], | |
searchFields: ['title^3', 'description'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wylieconlon you got me curious about this one – I investigated a bit.
Turns out that the request doesn't use this param from here, but takes it from:
https://github.com/elastic/kibana/blob/master/src/plugins/visualizations/public/saved_visualizations/find_list_items.ts#L55
We search through both lens and visualize so I think it's good . Example of request:
http://localhost:5603/clb/api/saved_objects/_find?default_search_operator=AND&page=1&per_page=1000&search=lens_vis*&search_fields=title%5E3&search_fields=description&type=lens&type=visualization
@elasticmachine merge upstream |
@wylieconlon what do you have in mind? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on FF, both functionality and code looks good to me 🆗
…ana into lens-duplicate-title-check
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
Fixes #64979
This PR uses the
checkForDuplicateTitle
helper function from thesaved_objects
plugin to make sure a Lens saved object is not saved with the same title as another existing saved object.As this helper was tied to the
SavedObject
type (which is not used by Lens), the type had to be adjusted slightly (src/plugins/saved_objects/public/saved_object/helpers/check_for_duplicate_title.ts
andsrc/plugins/saved_objects/public/saved_object/helpers/display_duplicate_title_confirm_modal.ts
)While working on this, I discovered there is a textarea for the description in the save modal, but Lens saved objects don't include a description at all. This PR also passes the description through the various layers so it's saved and shown correctly