-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[DataView] Keep only customLabel for data view minimal spec #178983
[DataView] Keep only customLabel for data view minimal spec #178983
Conversation
/ci |
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
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.
So I was playing around with this and I just wanted to make sure this is intentional behavior:
- Create an ad-hoc data view
- Add a field with a custom description
- Save the saved search
- Log out
- Log back in and open the saved search
The custom descriptions are not there, since they weren't persisted (b/c they're not part of the "minimal spec").
@@ -181,7 +181,7 @@ export class DataView extends AbstractDataView implements DataViewBase { | |||
if (dataViewSpec.fieldAttrs) { | |||
// removes `count` props (popularity scores) from `fieldAttrs` |
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.
Nit: can we update this comment to reflect that we're removing customDescription
as well?
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.
Thanks!
Changed the implementation to exclude custom description only for alert links. |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Public APIs missing comments
Page load bundle
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: cc @jughosta |
@@ -248,5 +248,5 @@ export function updateFilterReferences( | |||
export function getSmallerDataViewSpec( | |||
dataView: DataView | |||
): DiscoverAppLocatorParams['dataViewSpec'] { | |||
return dataView.toMinimalSpec(); | |||
return dataView.toMinimalSpec({ keepFieldAttrs: ['customLabel'] }); |
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.
Excluding customDescription
for alert links. customLabel
is what we allowed to include previously too.
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.
Code LGTM
Summary
We should probably exclude
customDescription
s from the data view minimal spec as they tend to be long.Checklist