search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.7k stars 8.12k forks source link

[Content management] make `managed` a required prop on the client #175157

Open drewdaemon opened 7 months ago

drewdaemon commented 7 months ago

The root saved object managed prop is declared as optional in core's server-side types to allow core to set the default of false.

https://github.com/elastic/kibana/blob/main/packages/core/saved-objects/core-saved-objects-common/src/server_types.ts#L111

However, by the time the SO is retrieved from the API, managed will always be present. But, we have allowed the optional nature of the saved object type to permeate both content management server code and Kibana client code.

https://github.com/elastic/kibana/blob/main/packages/kbn-content-management-utils/src/types.ts#L203

We should update the types to accurately depict the fact that managed is always present once a saved object is retrieved. I started down this path in https://github.com/elastic/kibana/pull/175062, but it's an effort in and of itself.

elasticmachine commented 7 months ago

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

elasticmachine commented 3 months ago

Pinging @elastic/kibana-presentation (Team:Presentation)

elasticmachine commented 3 months ago

Pinging @elastic/appex-sharedux (Team:SharedUX)

tsullivan commented 2 months ago

it's an effort in and of itself.

@drewdaemon can you give some context on this? Do you know the LOE of this task?

drewdaemon commented 2 months ago

@tsullivan I'm not sure on LOE. I don't think it's too bad, mostly splitting the interface into a client-side and a server-side version and importing them in the correct places.

It's just a typing inconsistency I noticed. I had to add a bunch of conditionals to check that "managed" was defined on the client when really it is always defined, the type just suggests that it isn't.